Skip to content

Commit 4dea38e

Browse files
rishipalcopybara-github
authored andcommitted
Assert that feature sets of scripts match before serialization and after deserialization in the SerializeAndDeserializeAstTest.java.
PiperOrigin-RevId: 731925529
1 parent c168082 commit 4dea38e

File tree

2 files changed

+61
-6
lines changed

2 files changed

+61
-6
lines changed

src/com/google/javascript/jscomp/serialization/SerializeTypedAstPass.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@
3030
/**
3131
* A compiler pass intended to serialize the types in the AST.
3232
*
33-
* <p>Under construction. Do not use!
33+
* <p>Serialization means that the AST is converted to a proto representation and output to a file
34+
* or stream. Deserialization means the reverse process.
3435
*/
3536
public final class SerializeTypedAstPass implements CompilerPass {
3637

test/com/google/javascript/jscomp/serialization/SerializeAndDeserializeAstTest.java

+59-5
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import static java.nio.charset.StandardCharsets.UTF_8;
2525

2626
import com.google.common.collect.ImmutableList;
27+
import com.google.common.collect.ImmutableMap;
2728
import com.google.common.collect.ImmutableSet;
2829
import com.google.javascript.jscomp.AbstractCompiler;
2930
import com.google.javascript.jscomp.AstValidator;
@@ -34,6 +35,7 @@
3435
import com.google.javascript.jscomp.SourceFile;
3536
import com.google.javascript.jscomp.colors.ColorRegistry;
3637
import com.google.javascript.jscomp.colors.StandardColors;
38+
import com.google.javascript.jscomp.parsing.parser.FeatureSet;
3739
import com.google.javascript.jscomp.serialization.TypedAstDeserializer.DeserializedAst;
3840
import com.google.javascript.rhino.IR;
3941
import com.google.javascript.rhino.InputId;
@@ -76,6 +78,7 @@ public final class SerializeAndDeserializeAstTest extends CompilerTestCase {
7678
private boolean parseInlineSourceMaps;
7779
private ImmutableList<String> runtimeLibraries = null;
7880
private Optional<PassFactory> preSerializePassFactory = Optional.empty();
81+
private boolean skipMatchingScriptFeaturesBeforeAndAfterSerialization = false;
7982

8083
@Override
8184
protected CompilerPass getProcessor(Compiler compiler) {
@@ -236,11 +239,13 @@ public void testCollapsePropertiesJsdoc() {
236239

237240
@Test
238241
public void testEmptyClassDeclaration() {
242+
skipMatchingScriptFeaturesBeforeAndAfterSerialization = true;
239243
testSame("class Foo {}");
240244
}
241245

242246
@Test
243247
public void testEmptyClassDeclarationWithExtends() {
248+
skipMatchingScriptFeaturesBeforeAndAfterSerialization = true;
244249
testSame("class Foo {} class Bar extends Foo {}");
245250
}
246251

@@ -264,6 +269,7 @@ public void testClassDeclarationWithMethods() {
264269

265270
@Test
266271
public void testClassDeclarationWithFields() {
272+
skipMatchingScriptFeaturesBeforeAndAfterSerialization = true;
267273
testSame(lines("class Foo {", " a = 1;", " d;", "}"));
268274

269275
// Type checking will report computed property accesses as errors for a class,
@@ -283,6 +289,7 @@ public void testClassDeclarationWithFields() {
283289

284290
@Test
285291
public void testEmptyClassStaticBlock() {
292+
skipMatchingScriptFeaturesBeforeAndAfterSerialization = true;
286293
testSame(
287294
lines(
288295
"class Foo {", //
@@ -293,6 +300,7 @@ public void testEmptyClassStaticBlock() {
293300

294301
@Test
295302
public void testClassStaticBlock_variables() {
303+
skipMatchingScriptFeaturesBeforeAndAfterSerialization = true;
296304
testSame(
297305
lines(
298306
"class Foo {", //
@@ -306,6 +314,7 @@ public void testClassStaticBlock_variables() {
306314

307315
@Test
308316
public void testClassStaticBlock_function() {
317+
skipMatchingScriptFeaturesBeforeAndAfterSerialization = true;
309318
testSame(
310319
lines(
311320
"class Foo {", //
@@ -318,6 +327,7 @@ public void testClassStaticBlock_function() {
318327

319328
@Test
320329
public void testMultipleClassStaticBlocks() {
330+
skipMatchingScriptFeaturesBeforeAndAfterSerialization = true;
321331
testSame(
322332
lines(
323333
"class Foo {", //
@@ -372,11 +382,13 @@ public void testYieldAll() {
372382

373383
@Test
374384
public void testFunctionCallRestAndSpread() {
385+
skipMatchingScriptFeaturesBeforeAndAfterSerialization = true;
375386
testSame("function f(...x) {} f(...[1, 2, 3]);");
376387
}
377388

378389
@Test
379390
public void testFunctionDefaultAndDestructuringParameters() {
391+
skipMatchingScriptFeaturesBeforeAndAfterSerialization = true;
380392
testSame("function f([a, b], x = 0, {y, ...z} = {y: 1}) {}");
381393
}
382394

@@ -806,10 +818,10 @@ private Result testAndReturnResult(Externs externs, Sources code, Expected expec
806818
InputStream serializedStream = toInputStream(externs, code, expected);
807819
Compiler serializingCompiler = getLastCompiler();
808820

809-
ImmutableList<SourceFile> externFiles =
810-
collectSourceFilesFromScripts(serializingCompiler.getRoot().getFirstChild());
811-
ImmutableList<SourceFile> codeFiles =
812-
collectSourceFilesFromScripts(serializingCompiler.getRoot().getSecondChild());
821+
Node oldRoot = serializingCompiler.getRoot();
822+
823+
ImmutableList<SourceFile> externFiles = collectSourceFilesFromScripts(oldRoot.getFirstChild());
824+
ImmutableList<SourceFile> codeFiles = collectSourceFilesFromScripts(oldRoot.getSecondChild());
813825

814826
// NOTE: We need a fresh compiler instance in which to deserialize, because:
815827
// 1. This is a better representation of what will happen in production use.
@@ -854,12 +866,54 @@ private Result testAndReturnResult(Externs externs, Sources code, Expected expec
854866

855867
Node expectedRoot = this.parseExpectedJs(expected);
856868
assertNode(newSourceRoot).isEqualIncludingJsDocTo(expectedRoot);
869+
Node newRoot = IR.root(newExternsRoot, newSourceRoot);
870+
857871
new AstValidator(deserializingCompiler, /* validateScriptFeatures= */ true)
858-
.validateRoot(IR.root(newExternsRoot, newSourceRoot));
872+
.validateRoot(newRoot);
873+
874+
// TODO(b/394454662): Remove this condition once feature set with deserialization and parser are
875+
// in sync.
876+
if (!skipMatchingScriptFeaturesBeforeAndAfterSerialization) {
877+
assertFeatureSetsOfScriptsMatch(oldRoot, newRoot);
878+
}
879+
859880
consumer = null;
860881
return new Result(ast, registry, newSourceRoot, deserializingCompiler);
861882
}
862883

884+
private void assertFeatureSetsOfScriptsMatch(Node oldRoot, Node newRoot) {
885+
ImmutableMap<SourceFile, FeatureSet> featureSetsOfOldScripts = getFeatureSetsOfScripts(oldRoot);
886+
ImmutableMap<SourceFile, FeatureSet> featureSetsOfNewScripts = getFeatureSetsOfScripts(newRoot);
887+
assertThat(featureSetsOfNewScripts.keySet()).isEqualTo(featureSetsOfOldScripts.keySet());
888+
for (SourceFile sourceFile : featureSetsOfOldScripts.keySet()) {
889+
FeatureSet oldFeatureSet = featureSetsOfOldScripts.get(sourceFile);
890+
FeatureSet newFeatureSet = featureSetsOfNewScripts.get(sourceFile);
891+
checkState(
892+
oldFeatureSet.equals(newFeatureSet),
893+
"Feature sets of scripts do not match for file %s. Old: %s, New: %s",
894+
sourceFile.getName(),
895+
oldFeatureSet,
896+
newFeatureSet);
897+
}
898+
}
899+
900+
private ImmutableMap<SourceFile, FeatureSet> getFeatureSetsOfScripts(Node root) {
901+
ImmutableMap.Builder<SourceFile, FeatureSet> builder = ImmutableMap.builder();
902+
Node externsRoot = root.getFirstChild();
903+
Node sourceRoot = root.getSecondChild();
904+
for (Node script = externsRoot.getFirstChild(); script != null; script = script.getNext()) {
905+
checkState(script.isScript());
906+
builder.put(
907+
(SourceFile) script.getStaticSourceFile(), (FeatureSet) script.getProp(Node.FEATURE_SET));
908+
}
909+
for (Node script = sourceRoot.getFirstChild(); script != null; script = script.getNext()) {
910+
checkState(script.isScript());
911+
builder.put(
912+
(SourceFile) script.getStaticSourceFile(), (FeatureSet) script.getProp(Node.FEATURE_SET));
913+
}
914+
return builder.buildOrThrow();
915+
}
916+
863917
private ImmutableList<SourceFile> collectSourceFilesFromScripts(Node root) {
864918
ImmutableList.Builder<SourceFile> files = ImmutableList.builder();
865919
for (Node n = root.getFirstChild(); n != null; n = n.getNext()) {

0 commit comments

Comments
 (0)