Skip to content

Commit 8418c02

Browse files
shickscopybara-github
shicks
authored and
copybara-github
committed
Allow boolean literals in _F_toggleOrdinals for compile-time-constant toggles.
PiperOrigin-RevId: 552635094
1 parent 87be3a5 commit 8418c02

File tree

2 files changed

+66
-30
lines changed

2 files changed

+66
-30
lines changed

‎src/com/google/javascript/jscomp/ReplaceToggles.java

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package com.google.javascript.jscomp;
1818

19+
import com.google.common.annotations.VisibleForTesting;
1920
import com.google.common.collect.ImmutableMap;
2021
import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
2122
import com.google.javascript.rhino.Node;
@@ -40,15 +41,22 @@ class ReplaceToggles implements CompilerPass {
4041
static final DiagnosticType INVALID_ORDINAL_MAPPING =
4142
DiagnosticType.error(
4243
"JSC_INVALID_ORDINAL_MAPPING",
43-
"_F_toggleOrdinals must be initialized with an object literal mapping strings to unique"
44-
+ " integers: {0}");
44+
"_F_toggleOrdinals must be initialized with an object literal mapping strings to booleans"
45+
+ " or unique whole numbers: {0}");
4546

4647
static final DiagnosticType UNKNOWN_TOGGLE =
4748
DiagnosticType.error(
4849
"JSC_UNKNOWN_TOGGLE",
4950
"goog.readToggleInternalDoNotCallDirectly called with an unknown toggle. If a toggle"
5051
+ " list is given, it must be exhaustive.");
5152

53+
// NOTE: These values are chosen as negative integers because actual toggle ordinals must always
54+
// be non-negative (at least zero). Any negative integers would do to distinguish them from real
55+
// toggle ordinals, but -1 and -2 are the simplest.
56+
@VisibleForTesting static final int TRUE_VALUE = -2;
57+
58+
@VisibleForTesting static final int FALSE_VALUE = -1;
59+
5260
private final AbstractCompiler compiler;
5361
private final AstFactory astFactory;
5462
private final boolean check;
@@ -100,17 +108,15 @@ public void visit(NodeTraversal t, Node n, Node parent) {
100108
compiler.report(JSError.make(c, INVALID_ORDINAL_MAPPING, "duplicate key: " + key));
101109
return;
102110
}
103-
Node value = c.getFirstChild();
104-
if (value == null || !value.isNumber()) {
105-
compiler.report(
106-
JSError.make(c, INVALID_ORDINAL_MAPPING, "value not a whole number literal"));
107-
return;
108-
}
109-
double doubleValue = value.getDouble();
110-
int intValue = (int) doubleValue;
111-
if (doubleValue != intValue) {
111+
Node child = c.getFirstChild();
112+
Double doubleValue = NodeUtil.getNumberValue(child);
113+
int intValue = doubleValue != null ? doubleValue.intValue() : -1;
114+
if (child.isTrue() || child.isFalse()) {
115+
intValue = child.isTrue() ? TRUE_VALUE : FALSE_VALUE;
116+
} else if (!child.isNumber() || intValue < 0 || intValue != doubleValue) {
112117
compiler.report(
113-
JSError.make(c, INVALID_ORDINAL_MAPPING, "value not a whole number literal"));
118+
JSError.make(
119+
c, INVALID_ORDINAL_MAPPING, "value not a boolean or whole number literal"));
114120
return;
115121
} else if (ordinals.contains(intValue)) {
116122
compiler.report(
@@ -150,9 +156,12 @@ public void visit(NodeTraversal t, Node n, Node parent) {
150156
}
151157

152158
Integer ordinal = toggles != null ? toggles.get(arg.getString()) : null;
153-
if (ordinal == null) {
154-
// No ordinals given: hard-code `false` for all toggles
155-
n.replaceWith(astFactory.createBoolean(false).srcrefTreeIfMissing(n));
159+
if (ordinal == null || ordinal < 0) {
160+
// No ordinals given: hard-code `true` if explicitly set as true, or `false` otherwise.
161+
n.replaceWith(
162+
astFactory
163+
.createBoolean(ordinal != null && ordinal == TRUE_VALUE)
164+
.srcrefTreeIfMissing(n));
156165
t.reportCodeChange();
157166
return;
158167
}

‎test/com/google/javascript/jscomp/ReplaceTogglesTest.java

Lines changed: 42 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
package com.google.javascript.jscomp;
1818

1919
import static com.google.common.truth.Truth.assertThat;
20+
import static com.google.javascript.jscomp.ReplaceToggles.FALSE_VALUE;
21+
import static com.google.javascript.jscomp.ReplaceToggles.TRUE_VALUE;
2022

2123
import com.google.common.collect.ImmutableMap;
2224
import org.junit.Before;
@@ -77,6 +79,25 @@ public void testBootstrapOrdinals_ignoresExtraUninitializedDefinitions() {
7779
expectOrdinals(ImmutableMap.of("foo", 1)));
7880
}
7981

82+
@Test
83+
public void testBootstrapOrdinals_booleanAllowed() {
84+
check = true;
85+
testSame(
86+
srcs("var _F_toggleOrdinals = {'foo_bar': false, 'baz': true};"),
87+
expectOrdinals(ImmutableMap.of("foo_bar", FALSE_VALUE, "baz", TRUE_VALUE)));
88+
}
89+
90+
@Test
91+
public void testBootstrapOrdinals_booleanAllowsDuplicates() {
92+
check = true;
93+
testSame(
94+
srcs("var _F_toggleOrdinals = {'foo_bar': false, 'baz': false, 'qux': 0};"),
95+
expectOrdinals(ImmutableMap.of("foo_bar", FALSE_VALUE, "baz", FALSE_VALUE, "qux", 0)));
96+
testSame(
97+
srcs("var _F_toggleOrdinals = {'foo_bar': true, 'baz': true, 'qux': 0};"),
98+
expectOrdinals(ImmutableMap.of("foo_bar", TRUE_VALUE, "baz", TRUE_VALUE, "qux", 0)));
99+
}
100+
80101
@Test
81102
public void testBootstrapOrdinals_notAnObject() {
82103
check = true;
@@ -115,42 +136,38 @@ public void testBootstrapOrdinals_duplicateKey() {
115136
@Test
116137
public void testBootstrapOrdinals_badValue() {
117138
check = true;
118-
test(
119-
srcs("var _F_toggleOrdinals = {x: null};"),
120-
error(ReplaceToggles.INVALID_ORDINAL_MAPPING)
121-
.withMessageContaining("value not a whole number literal"));
122139
test(
123140
srcs("var _F_toggleOrdinals = {x: 'y'};"),
124141
error(ReplaceToggles.INVALID_ORDINAL_MAPPING)
125-
.withMessageContaining("value not a whole number literal"));
142+
.withMessageContaining("value not a boolean or whole number literal"));
126143
test(
127144
srcs("var _F_toggleOrdinals = {x};"),
128145
error(ReplaceToggles.INVALID_ORDINAL_MAPPING)
129-
.withMessageContaining("value not a whole number literal"));
130-
test(
131-
srcs("var _F_toggleOrdinals = {x: true};"),
132-
error(ReplaceToggles.INVALID_ORDINAL_MAPPING)
133-
.withMessageContaining("value not a whole number literal"));
146+
.withMessageContaining("value not a boolean or whole number literal"));
134147
test(
135148
srcs("var _F_toggleOrdinals = {x: 1 + 2};"),
136149
error(ReplaceToggles.INVALID_ORDINAL_MAPPING)
137-
.withMessageContaining("value not a whole number literal"));
150+
.withMessageContaining("value not a boolean or whole number literal"));
138151
test(
139152
srcs("var _F_toggleOrdinals = {x: NaN};"),
140153
error(ReplaceToggles.INVALID_ORDINAL_MAPPING)
141-
.withMessageContaining("value not a whole number literal"));
154+
.withMessageContaining("value not a boolean or whole number literal"));
142155
test(
143156
srcs("var _F_toggleOrdinals = {x: Infinity};"),
144157
error(ReplaceToggles.INVALID_ORDINAL_MAPPING)
145-
.withMessageContaining("value not a whole number literal"));
158+
.withMessageContaining("value not a boolean or whole number literal"));
146159
test(
147160
srcs("var _F_toggleOrdinals = {x: -1};"),
148161
error(ReplaceToggles.INVALID_ORDINAL_MAPPING)
149-
.withMessageContaining("value not a whole number literal"));
162+
.withMessageContaining("value not a boolean or whole number literal"));
150163
test(
151164
srcs("var _F_toggleOrdinals = {x: 1.2};"),
152165
error(ReplaceToggles.INVALID_ORDINAL_MAPPING)
153-
.withMessageContaining("value not a whole number literal"));
166+
.withMessageContaining("value not a boolean or whole number literal"));
167+
test(
168+
srcs("var _F_toggleOrdinals = {x: null};"),
169+
error(ReplaceToggles.INVALID_ORDINAL_MAPPING)
170+
.withMessageContaining("value not a boolean or whole number literal"));
154171
}
155172

156173
@Test
@@ -183,6 +200,16 @@ public void testSimpleToggles() {
183200
"const baz = !!(goog.TOGGLES_[1] >> 29 & 1);"));
184201
}
185202

203+
@Test
204+
public void testUnsetToggles() {
205+
toggles = ImmutableMap.of("foo", FALSE_VALUE, "bar", TRUE_VALUE);
206+
test(
207+
lines(
208+
"const foo = goog.readToggleInternalDoNotCallDirectly('foo');",
209+
"const bar = goog.readToggleInternalDoNotCallDirectly('bar');"),
210+
lines("const foo = false;", "const bar = true;"));
211+
}
212+
186213
@Test
187214
public void testCheckMode() {
188215
check = true;

0 commit comments

Comments
 (0)