Skip to content

Commit 9e0a81f

Browse files
nanazecopybara-github
authored andcommitted
The class name to count map is never used, only its keys are. Replace with a set.
PiperOrigin-RevId: 551870873
1 parent 1f6e927 commit 9e0a81f

File tree

7 files changed

+42
-63
lines changed

7 files changed

+42
-63
lines changed

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@
5050
import java.nio.file.Paths;
5151
import java.util.ArrayList;
5252
import java.util.EnumSet;
53-
import java.util.LinkedHashMap;
5453
import java.util.List;
5554
import java.util.Set;
5655
import org.jspecify.nullness.Nullable;
@@ -129,7 +128,7 @@ void afterPass(String passName) {}
129128
public abstract void setStringMap(VariableMap stringMap);
130129

131130
/** Sets the css names found during compilation. */
132-
public abstract void setCssNames(LinkedHashMap<String, Integer> newCssNames);
131+
public abstract void setCssNames(Set<String> newCssNames);
133132

134133
/** Sets the mapping for instrumentation parameter encoding. */
135134
public abstract void setInstrumentationMapping(VariableMap instrumentationMapping);

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3507,11 +3507,8 @@ public SourceMap getSourceMap() {
35073507
/** Ids for cross-module method stubbing, so that each method has a unique id. */
35083508
private IdGenerator crossModuleIdGenerator = new IdGenerator();
35093509

3510-
/**
3511-
* Keys are arguments passed to getCssName() found during compilation; values are the number of
3512-
* times the key appeared as an argument to getCssName().
3513-
*/
3514-
private @Nullable LinkedHashMap<String, Integer> cssNames = null;
3510+
/** Arguments passed to getCssName() found during compilation. */
3511+
private @Nullable Set<String> cssNames = null;
35153512

35163513
/** The variable renaming map */
35173514
private @Nullable VariableMap variableMap = null;
@@ -3558,7 +3555,7 @@ public void setStringMap(VariableMap stringMap) {
35583555
}
35593556

35603557
@Override
3561-
public void setCssNames(LinkedHashMap<String, Integer> cssNames) {
3558+
public void setCssNames(Set<String> cssNames) {
35623559
this.cssNames = cssNames;
35633560
}
35643561

@@ -3960,7 +3957,7 @@ protected static class CompilerState implements Serializable {
39603957
private final int uniqueNameId;
39613958
private final UniqueIdSupplier uniqueIdSupplier;
39623959
private final LinkedHashSet<String> exportedNames;
3963-
private final LinkedHashMap<String, Integer> cssNames;
3960+
private final Set<String> cssNames;
39643961
private final String idGeneratorMap;
39653962
private final boolean transpiledFiles;
39663963
private final IdGenerator crossModuleIdGenerator;

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@
7676
import java.util.Collection;
7777
import java.util.HashMap;
7878
import java.util.HashSet;
79-
import java.util.LinkedHashMap;
8079
import java.util.List;
8180
import java.util.Map;
81+
import java.util.Optional;
8282
import java.util.Set;
8383
import org.jspecify.nullness.Nullable;
8484

@@ -1612,18 +1612,20 @@ compiler, new GoogleJsMessageIdGenerator(options.tcProjectId)
16121612
new CompilerPass() {
16131613
@Override
16141614
public void process(Node externs, Node jsRoot) {
1615-
LinkedHashMap<String, Integer> newCssNames = null;
1616-
if (options.gatherCssNames) {
1617-
newCssNames = new LinkedHashMap<>();
1618-
}
1615+
Optional<ImmutableSet.Builder<String>> cssNames =
1616+
options.gatherCssNames
1617+
? Optional.of(ImmutableSet.builder())
1618+
: Optional.empty();
1619+
16191620
ReplaceCssNames pass =
16201621
new ReplaceCssNames(
16211622
compiler,
16221623
options.cssRenamingMap,
1623-
newCssNames,
1624+
cssName -> cssNames.ifPresent(builder -> builder.add(cssName)),
16241625
options.cssRenamingSkiplist);
16251626
pass.process(externs, jsRoot);
1626-
compiler.setCssNames(newCssNames);
1627+
1628+
compiler.setCssNames(cssNames.isPresent() ? cssNames.get().build() : null);
16271629
}
16281630
})
16291631
.build();

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

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ class ReplaceCssNames implements CompilerPass {
140140
private final AbstractCompiler compiler;
141141
private final AstFactory astFactory;
142142

143-
private final Map<String, Integer> cssNames;
143+
private final CssNameCollector cssNameCollector;
144144

145145
private final Map<String, String> cssNamesBySymbol;
146146

@@ -150,15 +150,21 @@ class ReplaceCssNames implements CompilerPass {
150150

151151
private final Set<String> skiplist;
152152

153+
/** Called for each class name seen when replacing. */
154+
@FunctionalInterface
155+
public interface CssNameCollector {
156+
void add(String className);
157+
}
158+
153159
ReplaceCssNames(
154160
AbstractCompiler compiler,
155161
@Nullable CssRenamingMap symbolMap,
156-
@Nullable Map<String, Integer> cssNames,
162+
CssNameCollector cssNameCollector,
157163
@Nullable Set<String> skiplist) {
158164
this.compiler = compiler;
159165
this.astFactory = compiler.createAstFactory();
160166
this.symbolMap = symbolMap;
161-
this.cssNames = cssNames;
167+
this.cssNameCollector = cssNameCollector;
162168
this.skiplist = skiplist;
163169
this.cssNamesBySymbol = new LinkedHashMap<>();
164170
this.classesObjectsQualifiedNames = new LinkedHashSet<>();
@@ -322,12 +328,9 @@ private class TraversalState {
322328
* goog.getCssName('myComponent-highlight') will also work.
323329
*/
324330
private void updateCssNamesCount(String cssClassName) {
325-
if (cssNames != null) {
326-
String[] parts = cssClassName.split("-", -1);
327-
for (String element : parts) {
328-
int count = cssNames.getOrDefault(element, 0);
329-
cssNames.put(element, count + 1);
330-
}
331+
String[] parts = cssClassName.split("-", -1);
332+
for (String element : parts) {
333+
cssNameCollector.add(element);
331334
}
332335
}
333336

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
import com.google.common.annotations.VisibleForTesting;
2020
import com.google.common.collect.ImmutableList;
2121
import com.google.common.collect.ImmutableMap;
22-
import java.util.Map;
22+
import java.util.Set;
2323
import org.jspecify.nullness.Nullable;
2424

2525
/** Compilation results */
@@ -34,7 +34,7 @@ public class Result {
3434
public final VariableMap stringMap;
3535
public final VariableMap instrumentationMappings;
3636
public final SourceMap sourceMap;
37-
public final Map<String, Integer> cssNames;
37+
public final Set<String> cssNames;
3838
public final String externExport;
3939
public final String idGeneratorMap;
4040
public final boolean transpiledFiles;
@@ -49,7 +49,7 @@ public class Result {
4949
@Nullable VariableMap instrumentationMappings,
5050
@Nullable SourceMap sourceMap,
5151
String externExport,
52-
@Nullable Map<String, Integer> cssNames,
52+
@Nullable Set<String> cssNames,
5353
@Nullable String idGeneratorMap,
5454
boolean transpiledFiles) {
5555
this.success = errors.isEmpty();

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

Lines changed: 12 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@
2626

2727
import com.google.common.collect.ImmutableMap;
2828
import com.google.common.collect.ImmutableSet;
29+
import com.google.common.collect.Sets;
2930
import com.google.javascript.rhino.Node;
30-
import java.util.HashMap;
3131
import java.util.Map;
3232
import java.util.Set;
3333
import org.junit.Before;
@@ -68,7 +68,7 @@ public final class ReplaceCssNamesTest extends CompilerTestCase {
6868
CssRenamingMap renamingMap;
6969
Set<String> skiplist;
7070

71-
Map<String, Integer> cssNames;
71+
Set<String> cssNames;
7272

7373
public ReplaceCssNamesTest() {
7474
super(
@@ -82,7 +82,7 @@ public ReplaceCssNamesTest() {
8282
@Override
8383
protected CompilerPass getProcessor(Compiler compiler) {
8484
return new ReplaceCssNames(
85-
compiler, useReplacementMap ? renamingMap : null, cssNames, skiplist);
85+
compiler, useReplacementMap ? renamingMap : null, cssNames::add, skiplist);
8686
}
8787

8888
CssRenamingMap getPartialMap() {
@@ -109,7 +109,7 @@ public void setUp() throws Exception {
109109
super.setUp();
110110
enableTypeCheck();
111111
enableRewriteClosureCode();
112-
cssNames = new HashMap<>();
112+
cssNames = Sets.newHashSet();
113113
useReplacementMap = true;
114114
renamingMap = getPartialMap();
115115
}
@@ -179,16 +179,8 @@ public void testDoNotUseReplacementMap() {
179179
test(
180180
"setClass(goog.getCssName('active-buttonbar'))", //
181181
"setClass('active-buttonbar')");
182-
ImmutableMap<String, Integer> expected =
183-
new ImmutableMap.Builder<String, Integer>()
184-
.put("goog", 2)
185-
.put("footer", 1)
186-
.put("active", 2)
187-
.put("colorswatch", 1)
188-
.put("disabled", 1)
189-
.put("buttonbar", 1)
190-
.buildOrThrow();
191-
assertThat(cssNames).isEqualTo(expected);
182+
assertThat(cssNames)
183+
.containsExactly("goog", "footer", "active", "colorswatch", "disabled", "buttonbar");
192184
}
193185

194186
@Test
@@ -217,13 +209,7 @@ private void oneArgWithSimpleStringLiterals() {
217209
test(
218210
"setClass(goog.getCssName('elephant'))", //
219211
"setClass('e')");
220-
ImmutableMap<String, Integer> expected =
221-
new ImmutableMap.Builder<String, Integer>()
222-
.put("buttonbar", 1)
223-
.put("colorswatch", 1)
224-
.put("elephant", 1)
225-
.buildOrThrow();
226-
assertThat(cssNames).isEqualTo(expected);
212+
assertThat(cssNames).containsExactly("buttonbar", "colorswatch", "elephant");
227213
}
228214

229215
@Test
@@ -241,16 +227,8 @@ private void oneArgWithCompositeClassNames() {
241227
test(
242228
"setClass(goog.getCssName('active-buttonbar'))", //
243229
"setClass('a-b')");
244-
ImmutableMap<String, Integer> expected =
245-
new ImmutableMap.Builder<String, Integer>()
246-
.put("goog", 2)
247-
.put("footer", 1)
248-
.put("active", 2)
249-
.put("colorswatch", 1)
250-
.put("disabled", 1)
251-
.put("buttonbar", 1)
252-
.buildOrThrow();
253-
assertThat(cssNames).isEqualTo(expected);
230+
assertThat(cssNames)
231+
.containsExactly("goog", "footer", "active", "colorswatch", "disabled", "buttonbar");
254232
}
255233

256234
@Test
@@ -382,7 +360,7 @@ public void println(CheckLevel level, JSError error) {}
382360
compiler.setErrorManager(errorMan);
383361
Node root = compiler.parseTestCode(input);
384362
useReplacementMap = false;
385-
ReplaceCssNames replacer = new ReplaceCssNames(compiler, null, null, null);
363+
ReplaceCssNames replacer = new ReplaceCssNames(compiler, null, unused -> {}, null);
386364
replacer.process(null, root);
387365
assertThat(compiler.toSource(root)).isEqualTo("[\"test\",base+\"-active\"]");
388366
assertWithMessage("There should be no errors").that(errorMan.getErrorCount()).isEqualTo(0);
@@ -472,7 +450,7 @@ public void testVariableReferencesToCssClasses() {
472450
// name like this rather than creating an alias variable named `foo`.
473451
"var x = foo_styles_css.classes.Bar"));
474452
test(srcs(cssVarsDefinition, importer), expected(cssVarsExpected, importer));
475-
assertThat(cssNames).containsExactly("fooStylesBar", 1);
453+
assertThat(cssNames).containsExactly("fooStylesBar");
476454
}
477455

478456
@Test
@@ -524,7 +502,7 @@ public void testMixedVariableAndStringReferences() {
524502
"var x = foo_styles_css.classes.Bar;",
525503
"var y = 'a';"));
526504
test(srcs(cssVarsDefinition, importer), expected(cssVarsExpected, importerExpected));
527-
assertThat(cssNames).containsExactly("fooStylesBar", 1, "active", 1);
505+
assertThat(cssNames).containsExactly("fooStylesBar", "active");
528506
}
529507

530508
@Test

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1239,7 +1239,7 @@ public void testReplaceCssNames() {
12391239
+ "}",
12401240
"var COMPILED = true;\n" + "function getCss() {\n" + " return 'bar';" + "}");
12411241

1242-
assertThat(lastCompiler.getResult().cssNames).containsExactly("foo", 1);
1242+
assertThat(lastCompiler.getResult().cssNames).containsExactly("foo");
12431243
}
12441244

12451245
@Test

0 commit comments

Comments
 (0)