Skip to content

Commit c783340

Browse files
lauraharkercopybara-github
authored andcommitted
Make InferConsts stop treating all constant-by-convention variables as const
A variable that is actually provably either @const, uses the 'const' JS keyword, or is only assigned once at declaration is still treated as const. This is a step towards removing constant-by-convention completely. Remaining uses may be harder to remove: they affect CONSTANT_CASE properties, not just variables, and it's harder to statically detect constant properties than variables. PiperOrigin-RevId: 511930918
1 parent 47bef86 commit c783340

File tree

5 files changed

+39
-37
lines changed

5 files changed

+39
-37
lines changed

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
* 1) Provably well-defined and assigned once in its lifetime.
2727
* 2) Annotated 'const'
2828
* 3) Declared with the 'const' keyword.
29-
* 4) Is constant by naming convention.
3029
*
3130
* These 3 are considered semantically equivalent. Notice that a variable
3231
* in a loop is never considered const.
@@ -67,9 +66,6 @@ private void considerVar(Var v, @Nullable ReferenceCollection refCollection) {
6766
nameNode.setDeclaredConstantVar(true);
6867
} else if (nameNode != null && nameNode.getParent().isConst()) {
6968
nameNode.setDeclaredConstantVar(true);
70-
} else if (nameNode != null
71-
&& compiler.getCodingConvention().isConstant(nameNode.getString())) {
72-
nameNode.setDeclaredConstantVar(true);
7369
}
7470
if (isInferredConst(v, refCollection)) {
7571
nameNode.setInferredConstantVar(true);

src/com/google/javascript/jscomp/js/runtime_libs.typedast.textproto

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12147,7 +12147,7 @@ typed_asts {
1214712147
child {
1214812148
kind: IDENTIFIER
1214912149
string_value_pointer: 603
12150-
boolean_properties: 17592186044480
12150+
boolean_properties: 64
1215112151
type: 0
1215212152
}
1215312153
}
@@ -25656,7 +25656,6 @@ typed_asts {
2565625656
kind: IDENTIFIER
2565725657
string_value_pointer: 359
2565825658
relative_column: 4
25659-
boolean_properties: 17592186044416
2566025659
type: 314
2566125660
}
2566225661
relative_line: 7
@@ -118038,7 +118037,7 @@ typed_asts {
118038118037
}
118039118038
string_value_pointer: 1234
118040118039
relative_column: 4
118041-
boolean_properties: 26388279066624
118040+
boolean_properties: 8796093022208
118042118041
type: 3
118043118042
}
118044118043
relative_line: 1

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

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,12 @@ private void testWarning(String js){
5050

5151
@Test
5252
public void testConstantDefinition1() {
53-
testSame("var XYZ = 1;");
53+
testSame("/** @const */ var XYZ = 1;");
5454
}
5555

5656
@Test
5757
public void testConstantDefinition2() {
58-
testSame("var a$b$XYZ = 1;");
58+
testSame("/** @const */ var a$b$XYZ = 1;");
5959
}
6060

6161
@Test
@@ -71,27 +71,27 @@ public void testConstantDefinition4() {
7171

7272
@Test
7373
public void testConstantInitializedInAnonymousNamespace1() {
74-
testSame("var XYZ; (function(){ XYZ = 1; })();");
74+
testSame("/** @const */ var XYZ; (function(){ XYZ = 1; })();");
7575
}
7676

7777
@Test
7878
public void testConstantInitializedInAnonymousNamespace2() {
79-
testSame("var a$b$XYZ; (function(){ a$b$XYZ = 1; })();");
79+
testSame("/** @const */ var a$b$XYZ; (function(){ a$b$XYZ = 1; })();");
8080
}
8181

8282
@Test
8383
public void testObjectModified() {
84-
testSame("var IE = true, XYZ = {a:1,b:1}; if (IE) XYZ['c'] = 1;");
84+
testSame("/** @const */ var IE = true, XYZ = {a:1,b:1}; if (IE) XYZ['c'] = 1;");
8585
}
8686

8787
@Test
8888
public void testObjectPropertyInitializedLate() {
89-
testSame("var XYZ = {}; for (var i = 0; i < 10; i++) { XYZ[i] = i; }");
89+
testSame("/** @const */ var XYZ = {}; for (var i = 0; i < 10; i++) { XYZ[i] = i; }");
9090
}
9191

9292
@Test
9393
public void testObjectRedefined1() {
94-
testError("var XYZ = {}; XYZ = 2;");
94+
testError("/** @const */ var XYZ = {}; XYZ = 2;");
9595
}
9696

9797
@Test
@@ -101,12 +101,12 @@ public void testObjectRedefined2() {
101101

102102
@Test
103103
public void testConstantRedefined1() {
104-
testError("var XYZ = 1; XYZ = 2;");
104+
testError("/** @const */ var XYZ = 1; XYZ = 2;");
105105
}
106106

107107
@Test
108108
public void testConstantRedefined2() {
109-
testError("var a$b$XYZ = 1; a$b$XYZ = 2;");
109+
testError("/** @const */ var a$b$XYZ = 1; a$b$XYZ = 2;");
110110
}
111111

112112
// test will be caught be earlier pass, but demonstrates that it returns error upon const
@@ -118,62 +118,62 @@ public void testConstantRedefined3() {
118118

119119
@Test
120120
public void testConstantRedefined4() {
121-
testError("let XYZ = 1; XYZ = 2;");
121+
testError("/** @const */ let XYZ = 1; XYZ = 2;");
122122
}
123123

124124
@Test
125125
public void testConstantRedefinedInLocalScope1() {
126-
testError("var XYZ = 1; (function(){ XYZ = 2; })();");
126+
testError("/** @const */ var XYZ = 1; (function(){ XYZ = 2; })();");
127127
}
128128

129129
@Test
130130
public void testConstantRedefinedInLocalScope2() {
131-
testError("var a$b$XYZ = 1; (function(){ a$b$XYZ = 2; })();");
131+
testError("/** @const */ var a$b$XYZ = 1; (function(){ a$b$XYZ = 2; })();");
132132
}
133133

134134
@Test
135135
public void testConstantRedefinedInLocalScopeOutOfOrder() {
136-
testError("function f() { XYZ = 2; } var XYZ = 1;");
136+
testError("function f() { XYZ = 2; } /** @const */ var XYZ = 1;");
137137
}
138138

139139
@Test
140140
public void testConstantPostIncremented1() {
141-
testError("var XYZ = 1; XYZ++;");
141+
testError("/** @const */ var XYZ = 1; XYZ++;");
142142
}
143143

144144
@Test
145145
public void testConstantPostIncremented2() {
146-
testError("var a$b$XYZ = 1; a$b$XYZ++;");
146+
testError("/** @const */ var a$b$XYZ = 1; a$b$XYZ++;");
147147
}
148148

149149
@Test
150150
public void testConstantPreIncremented1() {
151-
testError("var XYZ = 1; ++XYZ;");
151+
testError("/** @const */ var XYZ = 1; ++XYZ;");
152152
}
153153

154154
@Test
155155
public void testConstantPreIncremented2() {
156-
testError("var a$b$XYZ = 1; ++a$b$XYZ;");
156+
testError("/** @const */ var a$b$XYZ = 1; ++a$b$XYZ;");
157157
}
158158

159159
@Test
160160
public void testConstantPostDecremented1() {
161-
testError("var XYZ = 1; XYZ--;");
161+
testError("/** @const */ var XYZ = 1; XYZ--;");
162162
}
163163

164164
@Test
165165
public void testConstantPostDecremented2() {
166-
testError("var a$b$XYZ = 1; a$b$XYZ--;");
166+
testError("/** @const */ var a$b$XYZ = 1; a$b$XYZ--;");
167167
}
168168

169169
@Test
170170
public void testConstantPreDecremented1() {
171-
testError("var XYZ = 1; --XYZ;");
171+
testError("/** @const */ var XYZ = 1; --XYZ;");
172172
}
173173

174174
@Test
175175
public void testConstantPreDecremented2() {
176-
testError("var a$b$XYZ = 1; --a$b$XYZ;");
176+
testError("/** @const */ var a$b$XYZ = 1; --a$b$XYZ;");
177177
}
178178

179179
@Test
@@ -183,37 +183,37 @@ public void testConstantPreDecremented3() {
183183

184184
@Test
185185
public void testAbbreviatedArithmeticAssignment1() {
186-
testError("var XYZ = 1; XYZ += 2;");
186+
testError("/** @const */ var XYZ = 1; XYZ += 2;");
187187
}
188188

189189
@Test
190190
public void testAbbreviatedArithmeticAssignment2() {
191-
testError("var a$b$XYZ = 1; a$b$XYZ %= 2;");
191+
testError("/** @const */ var a$b$XYZ = 1; a$b$XYZ %= 2;");
192192
}
193193

194194
@Test
195195
public void testAbbreviatedArithmeticAssignment3() {
196-
testError("var a$b$XYZ = 1; a$b$XYZ **= 2;");
196+
testError("/** @const */ var a$b$XYZ = 1; a$b$XYZ **= 2;");
197197
}
198198

199199
@Test
200200
public void testAbbreviatedBitAssignment1() {
201-
testError("var XYZ = 1; XYZ |= 2;");
201+
testError("/** @const */ var XYZ = 1; XYZ |= 2;");
202202
}
203203

204204
@Test
205205
public void testAbbreviatedBitAssignment2() {
206-
testError("var a$b$XYZ = 1; a$b$XYZ &= 2;");
206+
testError("/** @const */ var a$b$XYZ = 1; a$b$XYZ &= 2;");
207207
}
208208

209209
@Test
210210
public void testAbbreviatedShiftAssignment1() {
211-
testError("var XYZ = 1; XYZ >>= 2;");
211+
testError("/** @const */ var XYZ = 1; XYZ >>= 2;");
212212
}
213213

214214
@Test
215215
public void testAbbreviatedShiftAssignment2() {
216-
testError("var a$b$XYZ = 1; a$b$XYZ <<= 2;");
216+
testError("/** @const */ var a$b$XYZ = 1; a$b$XYZ <<= 2;");
217217
}
218218

219219
@Test

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,14 @@ public void testEsModuleImports() {
240240
assertConsts("import x from './mod';", notInferred("x"));
241241
assertConsts("import {x as y, z} from './mod';", notInferred("y", "z"));
242242
assertConsts("import * as x from './mod';", notInferred("x"));
243-
assertConsts("import * as CONST_NAME from './mod';", declared("CONST_NAME"));
243+
assertNotConsts("import * as CONST_NAME from './mod';", "CONST_NAME");
244+
}
245+
246+
@Test
247+
public void testConstantByConventionButNotInPractice() {
248+
assertNotConsts("let CONST_NAME = 0; CONST_NAME++;", "CONST_NAME");
249+
assertNotConsts("var CONST_NAME = 0; CONST_NAME++;", "CONST_NAME");
250+
assertConsts("var CONST_NAME = 0;", inferred("CONST_NAME"));
244251
}
245252

246253
private void assertNotConsts(String js, String... names) {

test/com/google/javascript/jscomp/integration/IntegrationTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1567,7 +1567,7 @@ public void testDevirtualizeMethods() {
15671567
public void testCheckConsts() {
15681568
CompilerOptions options = createCompilerOptions();
15691569
options.setInlineConstantVars(true);
1570-
test(options, "var FOO = true; FOO = false", DiagnosticGroups.CONST);
1570+
test(options, "/** @const */ var FOO = true; FOO = false", DiagnosticGroups.CONST);
15711571
}
15721572

15731573
@Test

0 commit comments

Comments
 (0)