Skip to content

Commit 74a3d91

Browse files
Closure Teamcopybara-github
Closure Team
authored andcommitted
Implement behavior for public fields with the @nocollapse JsDoc.
CheckJsDoc will now throw `JSC_MISPLACED_ANNOTATION` on fields with a `@nocollapse` that are not static. Field assignments inside constructors with the `@nocollpase` JsDoc present will also throw `JSC_MISPLACED_ANNOTATION`. Unit tests have been added for these additions. This pass puts the team closer to enabling public fields for the compiler. PiperOrigin-RevId: 554887149
1 parent 10ea2e2 commit 74a3d91

File tree

2 files changed

+60
-5
lines changed

2 files changed

+60
-5
lines changed

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

+14-5
Original file line numberDiff line numberDiff line change
@@ -420,14 +420,23 @@ private void validateNoCollapse(Node n, JSDocInfo info) {
420420
}
421421
if (NodeUtil.isPrototypePropertyDeclaration(n.getParent())
422422
|| (n.getParent().isClassMembers() && !n.isStaticMember())) {
423-
reportMisplaced(n, "nocollapse", "This JSDoc has no effect on prototype properties.");
423+
reportMisplaced(
424+
n,
425+
"nocollapse",
426+
"This JSDoc has no effect on prototype properties and non-static fields.");
427+
}
428+
if (n.isAssign()) {
429+
final Node assignee = n.getFirstChild();
430+
if (assignee.isQualifiedName()) {
431+
final Node rootOfQname = NodeUtil.getRootOfQualifiedName(assignee);
432+
if (!rootOfQname.isName()) {
433+
reportMisplaced(n, "nocollapse", "This JSDoc has no effect.");
434+
}
435+
}
424436
}
425437
}
426438

427-
/**
428-
* Checks that JSDoc intended for a function is actually attached to a
429-
* function.
430-
*/
439+
/** Checks that JSDoc intended for a function is actually attached to a function. */
431440
private void validateFunctionJsDoc(Node n, JSDocInfo info) {
432441
if (info == null) {
433442
return;

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

+46
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,52 @@ public void testInlineJsDocOnDeclaration() {
122122
testWarning("var /** {x: number} */ {x} = someObj;", CheckJSDoc.MISPLACED_ANNOTATION);
123123
}
124124

125+
@Test
126+
public void testFieldMisplacedAnnotation() {
127+
testWarning(
128+
lines(
129+
"class Foo {", //
130+
" /** @nocollapse */",
131+
" x = 5;",
132+
"}"),
133+
CheckJSDoc.MISPLACED_ANNOTATION);
134+
}
135+
136+
@Test
137+
public void testStaticFieldNoCollapse() {
138+
testSame(
139+
lines(
140+
"class Bar {", //
141+
" /** @nocollapse */",
142+
" static y = 1",
143+
"}"));
144+
}
145+
146+
@Test
147+
public void testThisPropertyMisplacedAnnotation() {
148+
testWarning(
149+
lines(
150+
"class Foo {", //
151+
" constructor() {",
152+
" /** @nocollapse */",
153+
" this.x = 4;",
154+
" }",
155+
"}"),
156+
CheckJSDoc.MISPLACED_ANNOTATION);
157+
}
158+
159+
@Test
160+
public void testThisInFunctionMisplacedAnnotation() {
161+
testWarning(
162+
lines(
163+
"/** @constructor */", //
164+
"function Bar() {",
165+
" /** @nocollapse */",
166+
" this.x = 4;",
167+
"}"),
168+
CheckJSDoc.MISPLACED_ANNOTATION);
169+
}
170+
125171
@Test
126172
public void testInvalidJsDocOnDestructuringDeclaration() {
127173
// Type annotations are not allowed on any declaration containing >=1 destructuring pattern.

0 commit comments

Comments
 (0)