Skip to content

Commit

Permalink
Allow public fields with the @final JsDoc to be assigned once insid…
Browse files Browse the repository at this point in the history
…e a constructor.

Originally, the `CheckAccessControls` pass did not support field assignment inside of a constructor and would throw the `JSC_FINAL_PROPERTY_OVERRIDEN` error. We now allow unassigned public fields with the `@final` JsDoc to be assigned once in the constructor. Allowing public fields to be recognized puts the team closer to flipping the flag to turn on public fields for google3.

PiperOrigin-RevId: 553601386
  • Loading branch information
Closure Team authored and copybara-github committed Aug 3, 2023
1 parent 4a040b4 commit 6e98e56
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/CheckAccessControls.java
Original file line number Diff line number Diff line change
Expand Up @@ -1368,7 +1368,7 @@ public final String getReadableTypeNameOrDefault() {
builder
.setName(sourceNode.getString())
.setReceiverType(typeRegistry.getNativeObjectType(JSTypeNative.UNKNOWN_TYPE))
.setMutation(true)
.setMutation(!(sourceNode.isMemberFieldDef() && !sourceNode.hasChildren()))
.setDeclaration(true)
// TODO(b/113704668): This definition is way too loose. It was used to prevent
// breakages during refactoring and should be tightened.
Expand Down
62 changes: 62 additions & 0 deletions test/com/google/javascript/jscomp/CheckAccessControlsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,68 @@ protected CompilerOptions getOptions() {
return options;
}

@Test
public void testOverrideAllowed() {
testSame(
lines(
"class Foo {", //
" /** @final*/ x;",
" constructor() {",
" this.x = 5;",
" }",
"}"));
}

@Test
public void testMultipleFieldsOverrideAllowed() {
testSame(
lines(
"class Bar {", //
" /** @final*/ a;",
" /** @final */ b;",
" constructor() {",
" this.a = 1;",
" this.b = 2;",
" }",
"}"));
}

@Test
public void testFieldOverrideNotAllowed() {
testError(
lines(
"class Bar {", //
" /** @final*/ a;",
" constructor() {",
" this.a = 1;",
" this.a = 2;",
" }",
"}"),
FINAL_PROPERTY_OVERRIDDEN);

testError(
lines(
"class Bar {", //
" /** @final*/ a = 5;",
" constructor() {",
" this.a = 1;",
" }",
"}"),
FINAL_PROPERTY_OVERRIDDEN);
}

@Test
public void testThisAssignmentInConstructor() {
testSame(
lines(
"class Foo {", //
" constructor() {",
" /** @const {number} */ this.x;",
" this.x = 4;",
" }",
"}"));
}

@Test
public void testWarningInNormalClass() {
test(
Expand Down

0 comments on commit 6e98e56

Please sign in to comment.