Skip to content

Commit 58f0fd4

Browse files
Closure Teamcopybara-github
authored andcommitted
Added public field this support to ClosureCheckModule for false positives.
Before, if a public field was assigned using the `this.property` format, ClosureCheckModule would throw `JSC_GOOG_MODULE_REFERENCES_THIS`. ClosureCheckModule now checks if the assignment is allowed given that the `this` is not inside a Goog module body which is done by updating the hoist scope. The unit tests demonstrate this new support feature for public fields. PiperOrigin-RevId: 551033593
1 parent a5b37b3 commit 58f0fd4

File tree

2 files changed

+47
-5
lines changed

2 files changed

+47
-5
lines changed

src/com/google/javascript/jscomp/NodeTraversal.java

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,11 +1100,7 @@ private void handleClassMembers(Node n, Node parent) {
11001100
popScope();
11011101
break;
11021102
case MEMBER_FIELD_DEF:
1103-
pushScope(child);
1104-
1105-
traverseBranch(child, n);
1106-
1107-
popScope();
1103+
handleMemberFieldDef(n, child);
11081104
break;
11091105
case BLOCK:
11101106
case MEMBER_FUNCTION_DEF:
@@ -1123,6 +1119,15 @@ private void handleClassMembers(Node n, Node parent) {
11231119
callback.visit(this, n, parent);
11241120
}
11251121

1122+
private void handleMemberFieldDef(Node n, Node child) {
1123+
Node previousHoistScopeRoot = currentHoistScopeRoot;
1124+
currentHoistScopeRoot = n;
1125+
pushScope(child);
1126+
traverseBranch(child, n);
1127+
popScope();
1128+
currentHoistScopeRoot = previousHoistScopeRoot;
1129+
}
1130+
11261131
private void traverseChildren(Node n) {
11271132
for (Node child = n.getFirstChild(); child != null; ) {
11281133
// child could be replaced, in which case our child node

test/com/google/javascript/jscomp/ClosureCheckModuleTest.java

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,43 @@ protected CompilerOptions getOptions() {
6666
return options;
6767
}
6868

69+
@Test
70+
public void testGoogModuleThisInSubclass() {
71+
testSame(
72+
lines(
73+
"goog.module('xyz');",
74+
"class Foo {", //
75+
" x = 5;",
76+
"}",
77+
"class Bar extends Foo {", //
78+
" z = this.x;",
79+
"}"));
80+
}
81+
82+
@Test
83+
public void testGoogModuleThisOnFields() {
84+
// Ensure `this.property` is allowed as an assignment on public fields.
85+
testSame(
86+
lines(
87+
"goog.module('xyz');",
88+
"class Foo {", //
89+
" x = 5;",
90+
" y = this.x",
91+
"}"));
92+
}
93+
94+
@Test
95+
public void testStaticGoogModuleThisOnStaticFields() {
96+
// Allow this to be allowed when a field is already declared
97+
testSame(
98+
lines(
99+
"goog.module('math')", //
100+
"class Foo {",
101+
" static x = 5;",
102+
" static y = this.x",
103+
"}"));
104+
}
105+
69106
@Test
70107
public void testGoogModuleReferencesThis() {
71108
testError("goog.module('xyz');\nfoo.call(this, 1, 2, 3);", GOOG_MODULE_REFERENCES_THIS);

0 commit comments

Comments
 (0)