Skip to content

Commit a0a2de2

Browse files
brad4dcopybara-github
authored andcommitted
Use LinkedHash(Map|Set) to guarantee deterministic iteration order
lint PiperOrigin-RevId: 621293367
1 parent 24a5352 commit a0a2de2

9 files changed

+79
-74
lines changed

src/com/google/javascript/jscomp/lint/CheckConstPrivateProperties.java

+6-4
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
import com.google.javascript.rhino.JSDocInfo.Visibility;
2727
import com.google.javascript.rhino.Node;
2828
import java.util.ArrayList;
29-
import java.util.HashSet;
29+
import java.util.LinkedHashSet;
3030
import java.util.List;
3131
import java.util.Set;
3232

@@ -42,8 +42,8 @@ public class CheckConstPrivateProperties extends AbstractPostOrderCallback imple
4242

4343
private final AbstractCompiler compiler;
4444
private final List<Node> candidates = new ArrayList<>();
45-
private final Set<String> modified = new HashSet<>();
46-
private final HashSet<String> constructorsAndInterfaces = new HashSet<>();
45+
private final Set<String> modified = new LinkedHashSet<>();
46+
private final LinkedHashSet<String> constructorsAndInterfaces = new LinkedHashSet<>();
4747

4848
public CheckConstPrivateProperties(AbstractCompiler compiler) {
4949
this.compiler = compiler;
@@ -143,7 +143,9 @@ private boolean isCandidatePropertyDefinition(Node n) {
143143
&& !isFunctionProperty(n);
144144
}
145145

146-
/** @return Whether the given property declaration is assigned to a function. */
146+
/**
147+
* @return Whether the given property declaration is assigned to a function.
148+
*/
147149
private boolean isFunctionProperty(Node n) {
148150
// TODO(dylandavidson): getAssignedValue does not support GETELEM.
149151
if (n.isGetElem()) {

src/com/google/javascript/jscomp/lint/CheckDuplicateCase.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
import com.google.javascript.jscomp.NodeTraversal;
2222
import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
2323
import com.google.javascript.rhino.Node;
24-
import java.util.HashSet;
24+
import java.util.LinkedHashSet;
2525
import java.util.Set;
2626

2727
/**
@@ -33,8 +33,8 @@
3333
* (https://github.com/eslint/eslint/blob/master/lib/rules/no-duplicate-case.js)
3434
*/
3535
public final class CheckDuplicateCase extends AbstractPostOrderCallback implements CompilerPass {
36-
public static final DiagnosticType DUPLICATE_CASE = DiagnosticType.warning(
37-
"JSC_DUPLICATE_CASE", "Duplicate case in a switch statement.");
36+
public static final DiagnosticType DUPLICATE_CASE =
37+
DiagnosticType.warning("JSC_DUPLICATE_CASE", "Duplicate case in a switch statement.");
3838

3939
private final AbstractCompiler compiler;
4040

@@ -50,7 +50,7 @@ public void process(Node externs, Node root) {
5050
@Override
5151
public void visit(NodeTraversal t, Node n, Node parent) {
5252
if (n.isSwitch()) {
53-
Set<String> cases = new HashSet<>();
53+
Set<String> cases = new LinkedHashSet<>();
5454
for (Node curr = n.getSecondChild(); curr != null; curr = curr.getNext()) {
5555
String source = compiler.toSource(curr.getFirstChild());
5656
if (!cases.add(source)) {

src/com/google/javascript/jscomp/lint/CheckEnums.java

+13-14
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
import com.google.javascript.rhino.JSDocInfo;
2727
import com.google.javascript.rhino.JSTypeExpression;
2828
import com.google.javascript.rhino.Node;
29-
import java.util.HashSet;
29+
import java.util.LinkedHashSet;
3030
import java.util.Set;
3131

3232
/**
@@ -39,18 +39,17 @@
3939
* </ol>
4040
*/
4141
public final class CheckEnums extends AbstractPostOrderCallback implements CompilerPass {
42-
public static final DiagnosticType DUPLICATE_ENUM_VALUE = DiagnosticType.disabled(
43-
"JSC_DUPLICATE_ENUM_VALUE",
44-
"The value {0} is duplicated in this enum.");
45-
public static final DiagnosticType COMPUTED_PROP_NAME_IN_ENUM = DiagnosticType.disabled(
46-
"JSC_COMPUTED_PROP_NAME_IN_ENUM",
47-
"Computed property name used in enum.");
48-
public static final DiagnosticType SHORTHAND_ASSIGNMENT_IN_ENUM = DiagnosticType.disabled(
49-
"JSC_SHORTHAND_ASSIGNMENT_IN_ENUM",
50-
"Shorthand assignment used in enum.");
51-
public static final DiagnosticType ENUM_PROP_NOT_CONSTANT = DiagnosticType.disabled(
52-
"JSC_ENUM_PROP_NOT_CONSTANT",
53-
"enum key {0} must be in ALL_CAPS.");
42+
public static final DiagnosticType DUPLICATE_ENUM_VALUE =
43+
DiagnosticType.disabled(
44+
"JSC_DUPLICATE_ENUM_VALUE", "The value {0} is duplicated in this enum.");
45+
public static final DiagnosticType COMPUTED_PROP_NAME_IN_ENUM =
46+
DiagnosticType.disabled(
47+
"JSC_COMPUTED_PROP_NAME_IN_ENUM", "Computed property name used in enum.");
48+
public static final DiagnosticType SHORTHAND_ASSIGNMENT_IN_ENUM =
49+
DiagnosticType.disabled(
50+
"JSC_SHORTHAND_ASSIGNMENT_IN_ENUM", "Shorthand assignment used in enum.");
51+
public static final DiagnosticType ENUM_PROP_NOT_CONSTANT =
52+
DiagnosticType.disabled("JSC_ENUM_PROP_NOT_CONSTANT", "enum key {0} must be in ALL_CAPS.");
5453

5554
public static final DiagnosticType ENUM_TYPE_NOT_STRING_OR_NUMBER =
5655
DiagnosticType.disabled(
@@ -138,7 +137,7 @@ private void checkName(NodeTraversal t, Node prop) {
138137
}
139138

140139
private static void checkDuplicateEnumValues(NodeTraversal t, Node enumNode) {
141-
Set<String> values = new HashSet<>();
140+
Set<String> values = new LinkedHashSet<>();
142141
for (Node prop = enumNode.getFirstChild(); prop != null; prop = prop.getNext()) {
143142
Node valueNode = prop.getLastChild();
144143
String value;

src/com/google/javascript/jscomp/lint/CheckEs6Modules.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
import com.google.javascript.jscomp.DiagnosticType;
2222
import com.google.javascript.jscomp.NodeTraversal;
2323
import com.google.javascript.rhino.Node;
24-
import java.util.HashMap;
24+
import java.util.LinkedHashMap;
2525
import java.util.Map;
2626

2727
/** Miscellaneous checks for style in ES6 modules. */
@@ -38,7 +38,7 @@ public final class CheckEs6Modules implements NodeTraversal.Callback, CompilerPa
3838
+ "imported.");
3939

4040
private final AbstractCompiler compiler;
41-
private final Map<String, Node> importSpecifiers = new HashMap<>();
41+
private final Map<String, Node> importSpecifiers = new LinkedHashMap<>();
4242

4343
public CheckEs6Modules(AbstractCompiler compiler) {
4444
this.compiler = compiler;

src/com/google/javascript/jscomp/lint/CheckExtraRequires.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@
2828
import com.google.javascript.rhino.JSDocInfo;
2929
import com.google.javascript.rhino.Node;
3030
import com.google.javascript.rhino.QualifiedName;
31-
import java.util.HashMap;
32-
import java.util.HashSet;
31+
import java.util.LinkedHashMap;
32+
import java.util.LinkedHashSet;
3333
import java.util.Map;
3434
import java.util.Set;
3535
import org.jspecify.nullness.Nullable;
@@ -43,12 +43,12 @@ public class CheckExtraRequires extends NodeTraversal.AbstractPostOrderCallback
4343
private final AbstractCompiler compiler;
4444

4545
// Keys are the local name of a required namespace. Values are the goog.require CALL node.
46-
private final Map<String, Node> requires = new HashMap<>();
46+
private final Map<String, Node> requires = new LinkedHashMap<>();
4747

4848
// Adding an entry to usages indicates that the name (either a fully qualified or local name)
4949
// is used and can be required. Note that since usages are name-based and not scoped, any usage
5050
// that shadows an unused require in that file will cause the extra require warning to be missed.
51-
private final Set<String> usages = new HashSet<>();
51+
private final Set<String> usages = new LinkedHashSet<>();
5252

5353
/**
5454
* This is only relevant for the standalone CheckExtraRequires run. This is used to restrict the

src/com/google/javascript/jscomp/lint/CheckGoogModuleTypeScriptName.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
import com.google.javascript.jscomp.NodeTraversal;
2323
import com.google.javascript.jscomp.NodeUtil;
2424
import com.google.javascript.rhino.Node;
25-
import java.util.HashSet;
25+
import java.util.LinkedHashSet;
2626
import java.util.Set;
2727

2828
/**
@@ -36,14 +36,15 @@ public final class CheckGoogModuleTypeScriptName implements NodeTraversal.Callba
3636
"goog.module namespace does not match the future TypeScript namespace, which is generated"
3737
+ " from the file path."
3838
+ " The correct namespace is: \"{0}\"");
39-
private static final Set<String> allowedDirectories = new HashSet<>();
39+
private static final Set<String> allowedDirectories = new LinkedHashSet<>();
4040

4141
// MOE::begin_strip
4242
static {
4343
allowedDirectories.add("google3/gws/");
4444
allowedDirectories.add("google3/java/com/google/gws/");
4545
allowedDirectories.add("google3/javascript/search/");
4646
}
47+
4748
// MOE::end_strip
4849

4950
private final AbstractCompiler compiler;

src/com/google/javascript/jscomp/lint/CheckNoMutatedEs6Exports.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
import com.google.javascript.jscomp.Scope;
3131
import com.google.javascript.jscomp.Var;
3232
import com.google.javascript.rhino.Node;
33-
import java.util.HashSet;
33+
import java.util.LinkedHashSet;
3434
import java.util.Set;
3535

3636
/** Checks that exports of ES6 modules are not mutated outside of module initialization. */
@@ -47,7 +47,7 @@ public final class CheckNoMutatedEs6Exports implements NodeTraversal.Callback, C
4747
private final AbstractCompiler compiler;
4848
private final Multimap<String, Node> mutatedNames =
4949
MultimapBuilder.hashKeys().hashSetValues().build();
50-
private final Set<String> exportedLocalNames = new HashSet<>();
50+
private final Set<String> exportedLocalNames = new LinkedHashSet<>();
5151

5252
public CheckNoMutatedEs6Exports(AbstractCompiler compiler) {
5353
this.compiler = compiler;

src/com/google/javascript/jscomp/lint/CheckNullabilityModifiers.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
import com.google.javascript.rhino.JSTypeExpression;
2828
import com.google.javascript.rhino.Node;
2929
import com.google.javascript.rhino.Token;
30-
import java.util.HashSet;
30+
import java.util.LinkedHashSet;
3131
import org.jspecify.nullness.Nullable;
3232

3333
/**
@@ -69,10 +69,10 @@ public class CheckNullabilityModifiers extends AbstractPostOrderCallback impleme
6969
private final AbstractCompiler compiler;
7070

7171
// Store the candidate warnings and template types found while traversing a single script node.
72-
private final HashSet<Node> redundantCandidates = new HashSet<>();
73-
private final HashSet<Node> missingCandidates = new HashSet<>();
74-
private final HashSet<Node> nullMissingCandidates = new HashSet<>();
75-
private final HashSet<String> templateTypeNames = new HashSet<>();
72+
private final LinkedHashSet<Node> redundantCandidates = new LinkedHashSet<>();
73+
private final LinkedHashSet<Node> missingCandidates = new LinkedHashSet<>();
74+
private final LinkedHashSet<Node> nullMissingCandidates = new LinkedHashSet<>();
75+
private final LinkedHashSet<String> templateTypeNames = new LinkedHashSet<>();
7676

7777
public CheckNullabilityModifiers(AbstractCompiler compiler) {
7878
this.compiler = compiler;

src/com/google/javascript/jscomp/lint/CheckUnusedPrivateProperties.java

+40-37
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
import com.google.javascript.rhino.JSDocInfo.Visibility;
2828
import com.google.javascript.rhino.Node;
2929
import java.util.ArrayList;
30-
import java.util.HashSet;
30+
import java.util.LinkedHashSet;
3131
import java.util.List;
3232
import java.util.Set;
3333

@@ -42,9 +42,9 @@ public class CheckUnusedPrivateProperties implements CompilerPass, NodeTraversal
4242
DiagnosticType.disabled("JSC_UNUSED_PRIVATE_PROPERTY", "Private property {0} is never read");
4343

4444
private final AbstractCompiler compiler;
45-
private final Set<String> used = new HashSet<>();
45+
private final Set<String> used = new LinkedHashSet<>();
4646
private final List<Node> candidates = new ArrayList<>();
47-
private final HashSet<String> constructorsAndInterfaces = new HashSet<>();
47+
private final LinkedHashSet<String> constructorsAndInterfaces = new LinkedHashSet<>();
4848

4949
public CheckUnusedPrivateProperties(AbstractCompiler compiler) {
5050
this.compiler = compiler;
@@ -90,55 +90,59 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
9090
@Override
9191
public void visit(NodeTraversal t, Node n, Node parent) {
9292
switch (n.getToken()) {
93-
case SCRIPT: {
94-
// exiting the script, report any privates not used in the file.
95-
reportUnused(t);
96-
break;
97-
}
93+
case SCRIPT:
94+
{
95+
// exiting the script, report any privates not used in the file.
96+
reportUnused(t);
97+
break;
98+
}
9899

99-
case GETPROP: {
100+
case GETPROP:
101+
{
100102
String propName = n.getString();
101103
if (isPinningPropertyUse(n) || !isCandidatePropertyDefinition(n)) {
102-
used.add(propName);
103-
} else {
104-
// Only consider "private" properties.
105-
if (isCheckablePrivatePropDecl(n)) {
106-
candidates.add(n);
107-
}
108-
}
109-
break;
110-
}
111-
112-
case MEMBER_FUNCTION_DEF: {
113-
// Only consider "private" methods.
114-
if (isCheckablePrivatePropDecl(n)) {
115-
candidates.add(n);
116-
}
117-
break;
118-
}
119-
120-
case OBJECTLIT: {
104+
used.add(propName);
105+
} else {
106+
// Only consider "private" properties.
107+
if (isCheckablePrivatePropDecl(n)) {
108+
candidates.add(n);
109+
}
110+
}
111+
break;
112+
}
113+
114+
case MEMBER_FUNCTION_DEF:
115+
{
116+
// Only consider "private" methods.
117+
if (isCheckablePrivatePropDecl(n)) {
118+
candidates.add(n);
119+
}
120+
break;
121+
}
122+
123+
case OBJECTLIT:
124+
{
121125
// Assume any object literal definition might be a reflection on the
122126
// class property.
123127
for (Node c = n.getFirstChild(); c != null; c = c.getNext()) {
124128
if (c.isStringKey() || c.isGetterDef() || c.isSetterDef() || c.isMemberFunctionDef()) {
125129
used.add(c.getString());
126130
}
127-
}
128-
break;
129-
}
131+
}
132+
break;
133+
}
130134

131135
case CALL:
132136
// Look for properties referenced through a property rename function.
133137
Node target = n.getFirstChild();
134138
if (n.hasMoreThanOneChild()
135139
&& compiler.getCodingConvention().isPropertyRenameFunction(target)) {
136-
Node propName = target.getNext();
140+
Node propName = target.getNext();
137141
if (propName.isStringLit()) {
138142
used.add(propName.getString());
139-
}
140-
}
141-
break;
143+
}
144+
}
145+
break;
142146

143147
case FUNCTION:
144148
JSDocInfo info = NodeUtil.getBestJSDocInfo(n);
@@ -205,8 +209,7 @@ private static boolean isPinningPropertyUse(Node n) {
205209
} else if (parent.isAssign()) {
206210
// A simple assignment doesn't pin the property.
207211
return false;
208-
} else if (NodeUtil.isAssignmentOp(parent)
209-
|| parent.isInc() || parent.isDec()) {
212+
} else if (NodeUtil.isAssignmentOp(parent) || parent.isInc() || parent.isDec()) {
210213
// In general, compound assignments are both reads and writes, but
211214
// if the property is never otherwise read we can consider it simply
212215
// a write.

0 commit comments

Comments
 (0)