SONARARMOR-860 Parser should serialize popular syntax #5257

roberto-orlandi-sonarsource · 2025-03-28T16:00:10Z

Part of

roberto-orlandi-sonarsource · 2025-03-31T05:49:41Z

packages/jsts/tests/parsers/ast.test.ts

@@ -262,15 +263,6 @@ describe('ast', () => {
    checkAstIsProperlySerializedAndDeserialized(ast as TSESTree.Program, protoMessage, 'foo.ts');
  });

-  test('Unknown node types in program body are not serialized', async () => {


This test was previously implemented to check that from a list of nodes we are able to filter out the unknown ones. With this PR we implement basic support for some nodes previously unsupported. After this, I could not find another test code to use to reproduce this scenario, so I removed this test, while I kept the business logic, as I am sure that it is still a possible scenario in production.

roberto-orlandi-sonarsource · 2025-03-31T05:51:16Z

sonar-plugin/api/src/main/java/org/sonar/plugins/javascript/api/estree/ESTree.java

@@ -673,7 +673,7 @@ public record TSModuleDeclaration(
    Optional<TSModuleBlock> body,
    String kind
  )
-    implements ModuleDeclaration {}
+    implements Declaration {}


This fixes an error in the hierarchy introduced during the previous PR to support TSModuleDeclaration module.

roberto-orlandi-sonarsource · 2025-03-31T05:55:03Z

...n/standalone/src/test/java/org/sonar/plugins/javascript/standalone/StandaloneParserTest.java

Inspired by this comment in the previous PR, I added some UTs in this class, to check that given a piece of code we create the expected java nodes.

roberto-orlandi-sonarsource · 2025-03-31T06:04:09Z

This PR contains the following:

Implement basic support for some of the most popular TS unsupported nodes, where basic support means that we serialize and deserialize them as empty objects, with the correct type. This will prevent us from stopping the analysis on SonarJasmin, when those nodes are encountered
Fixes an error introduced in the ESTree.java hierarchy during a previous PR: The TSModuleDeclaration should be a subclass of Declaration and not of ModuleDeclaration. This was spotted because we would fail handling code such as export namespace A {}, because the export syntax node expects a Declaration as a child.
A few more UTs added in the StandaloneParserTest, inspired by a previous PR's comment

zglicz · 2025-03-31T09:21:04Z

packages/jsts/tests/parsers/ast.test.ts

+  test('should serialize some nodes to empty object', async () => {
+    const codeMap = new Map<string, string>();
+    codeMap.set('TSTypeAliasDeclaration', `type A = { a: string };`);
+    codeMap.set('TSInterfaceDeclaration', `interface A { a: string; }`);
+    codeMap.set('TSEnumDeclaration', `enum Direction {}`);
+    codeMap.set('TSDeclareFunction', `declare function foo()`);
+
+    for (const [nodeType, code] of codeMap) {
+      const ast = await parseSourceCode(code, parsersMap.typescript);
+      const protoMessage = visitNode(ast as TSESTree.Program);
+
+      expect(protoMessage.program.body[0].type).toEqual(NODE_TYPE_ENUM.values[`${nodeType}Type`]);
+      const tSModuleDeclaration = protoMessage.program.body[0][lowerCaseFirstLetter(nodeType)];
+      expect(tSModuleDeclaration).toEqual({});
+      checkAstIsProperlySerializedAndDeserialized(ast as TSESTree.Program, protoMessage, 'foo.ts');
+    }
+  });


I think a better practice to implement would be to run the test in a loop. Sth like this:

Suggested change

test('should serialize some nodes to empty object', async () => {

const codeMap = new Map<string, string>();

codeMap.set('TSTypeAliasDeclaration', `type A = { a: string };`);

codeMap.set('TSInterfaceDeclaration', `interface A { a: string; }`);

codeMap.set('TSEnumDeclaration', `enum Direction {}`);

codeMap.set('TSDeclareFunction', `declare function foo()`);

for (const [nodeType, code] of codeMap) {

const ast = await parseSourceCode(code, parsersMap.typescript);

const protoMessage = visitNode(ast as TSESTree.Program);

expect(protoMessage.program.body[0].type).toEqual(NODE_TYPE_ENUM.values[`${nodeType}Type`]);

const tSModuleDeclaration = protoMessage.program.body[0][lowerCaseFirstLetter(nodeType)];

expect(tSModuleDeclaration).toEqual({});

checkAstIsProperlySerializedAndDeserialized(ast as TSESTree.Program, protoMessage, 'foo.ts');

}

});

[

{ nodeType: 'TSTypeAliasDeclaration', code: `type A = { a: string };` },

{ nodeType: 'TSInterfaceDeclaration', code: `interface A { a: string; }`},

{ nodeType: 'TSEnumDeclaration', code: `enum Direction {}`},

{ nodeType: 'TSDeclareFunction', code: `declare function foo()`},

].forEach(({ nodeType, code}) => test(`should serialize ${nodeType} to empty object`, async () => {

const ast = await parseSourceCode(code, parsersMap.typescript);

const protoMessage = visitNode(ast as TSESTree.Program);

expect(protoMessage.program.body[0].type).toEqual(NODE_TYPE_ENUM.values[`${nodeType}Type`]);

const tSModuleDeclaration = protoMessage.program.body[0][lowerCaseFirstLetter(nodeType)];

expect(tSModuleDeclaration).toEqual({});

checkAstIsProperlySerializedAndDeserialized(ast as TSESTree.Program, protoMessage, 'foo.ts');

})

});

Pardon if this is invalid js code, I was typing in this textbox freehand.
This way, you get the name of the node if it fails and its an actual unit-test.

Nice, thanks for the suggestion!

zglicz · 2025-03-31T09:22:24Z

sonar-plugin/api/src/test/java/org/sonar/plugins/javascript/api/estree/ESTreeTest.java

@@ -26,14 +26,14 @@ class ESTreeTest {
  @Test
  void test() {
    Class<?>[] classes = ESTree.class.getDeclaredClasses();
-    assertThat(classes).hasSize(118);
+    assertThat(classes).hasSize(125);


zglicz · 2025-03-31T09:25:07Z

sonar-plugin/bridge/src/test/java/org/sonar/plugins/javascript/bridge/ESTreeFactoryTest.java

+  @Test
+  void should_create_ts_abstract_method_definition() {
+    assertNodeTypeIsParsedToExpectedClass(
+      NodeType.TSAbstractMethodDefinitionType,
+      ESTree.TSAbstractMethodDefinition.class
+    );
+  }
+
+  @Test
+  void should_create_ts_declare_function() {
+    assertNodeTypeIsParsedToExpectedClass(
+      NodeType.TSDeclareFunctionType,
+      ESTree.TSDeclareFunction.class
+    );
+  }
+
+  @Test
+  void should_create_ts_interface_declaration() {
+    assertNodeTypeIsParsedToExpectedClass(
+      NodeType.TSInterfaceDeclarationType,
+      ESTree.TSInterfaceDeclaration.class
+    );
+  }
+
+  @Test
+  void should_create_ts_enum_declaration() {
+    assertNodeTypeIsParsedToExpectedClass(
+      NodeType.TSEnumDeclarationType,
+      ESTree.TSEnumDeclaration.class
+    );
+  }
+
+  @Test
+  void should_create_ts_type_alias_declaration() {
+    assertNodeTypeIsParsedToExpectedClass(
+      NodeType.TSTypeAliasDeclarationType,
+      ESTree.TSTypeAliasDeclaration.class
+    );
+  }
+
+  @Test
+  void should_create_ts_empty_body_function_expression() {
+    assertNodeTypeIsParsedToExpectedClass(
+      NodeType.TSEmptyBodyFunctionExpressionType,
+      ESTree.TSEmptyBodyFunctionExpression.class
+    );
+  }


I'm not fluent in Java, but couldn't this be a @parametrizedTest with these test cases provided with a @methodSource? No need naming these tests and they all share a common implementation from line 1013?

zglicz · 2025-03-31T09:27:05Z

...n/standalone/src/test/java/org/sonar/plugins/javascript/standalone/StandaloneParserTest.java

+  @Test
+  void should_parse_exported_ts_module_declaration() {
+    assertExportedNodeIsParsedToCorrectObjectType(
+      "export declare module 'foo'",
+      ESTree.TSModuleDeclaration.class
+    );
+  }
+
+  @Test
+  void should_parse_exported_ts_type_alias_declaration() {
+    assertExportedNodeIsParsedToCorrectObjectType(
+      "export type A = { a: 42 }",
+      ESTree.TSTypeAliasDeclaration.class
+    );
+  }
+
+  @Test
+  void should_parse_exported_ts_enum_declaration() {
+    assertExportedNodeIsParsedToCorrectObjectType(
+      "export enum A {}",
+      ESTree.TSEnumDeclaration.class
+    );
+  }
+
+  @Test
+  void should_parse_exported_ts_interface_declaration() {
+    assertExportedNodeIsParsedToCorrectObjectType(
+      "export interface A {}",
+      ESTree.TSInterfaceDeclaration.class
+    );
+  }
+
+  @Test
+  void should_parse_exported_ts_declare_function() {
+    assertExportedNodeIsParsedToCorrectObjectType(
+      "export declare function foo()",
+      ESTree.TSDeclareFunction.class
+    );
+  }


Same comment as before, could be parametrized to reduce code-bloat.

sonarqube-next · 2025-03-31T10:18:01Z

Quality Gate failed

Failed conditions
63.6% Coverage on New Code (required ≥ 90%)

See analysis details on SonarQube

SONARARMOR-860 Parser should serialize popular syntax

e2e8748

roberto-orlandi-sonarsource force-pushed the rob/SONARARMOR-860 branch from cb8fe29 to 64bf8b9 Compare March 30, 2025 19:01

improve

70eb540

roberto-orlandi-sonarsource force-pushed the rob/SONARARMOR-860 branch from 64bf8b9 to 70eb540 Compare March 30, 2025 19:16

fix-test

bf9683e

roberto-orlandi-sonarsource commented Mar 31, 2025

View reviewed changes

roberto-orlandi-sonarsource marked this pull request as ready for review March 31, 2025 06:04

roberto-orlandi-sonarsource requested a review from a team March 31, 2025 06:07

zglicz approved these changes Mar 31, 2025

View reviewed changes

roberto-orlandi-sonarsource added 2 commits March 31, 2025 12:03

review

abd615a

add empty lines

6a44ee8

roberto-orlandi-sonarsource merged commit 1216a05 into master Mar 31, 2025
16 of 17 checks passed

roberto-orlandi-sonarsource deleted the rob/SONARARMOR-860 branch March 31, 2025 10:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

SONARARMOR-860 Parser should serialize popular syntax #5257

SONARARMOR-860 Parser should serialize popular syntax #5257

Uh oh!

roberto-orlandi-sonarsource commented Mar 28, 2025 •

edited by jira bot

Loading

Uh oh!

roberto-orlandi-sonarsource Mar 31, 2025

Uh oh!

roberto-orlandi-sonarsource Mar 31, 2025

Uh oh!

roberto-orlandi-sonarsource Mar 31, 2025

Uh oh!

roberto-orlandi-sonarsource commented Mar 31, 2025

Uh oh!

zglicz Mar 31, 2025

Uh oh!

roberto-orlandi-sonarsource Mar 31, 2025

Uh oh!

zglicz Mar 31, 2025

Uh oh!

zglicz Mar 31, 2025

Uh oh!

roberto-orlandi-sonarsource Mar 31, 2025

Uh oh!

zglicz Mar 31, 2025

Uh oh!

sonarqube-next bot commented Mar 31, 2025

Uh oh!

Uh oh!

Uh oh!

SONARARMOR-860 Parser should serialize popular syntax #5257

SONARARMOR-860 Parser should serialize popular syntax #5257

Uh oh!

Conversation

roberto-orlandi-sonarsource commented Mar 28, 2025 • edited by jira bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roberto-orlandi-sonarsource commented Mar 31, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqube-next bot commented Mar 31, 2025

Quality Gate failed

Uh oh!

Uh oh!

Uh oh!

roberto-orlandi-sonarsource commented Mar 28, 2025 •

edited by jira bot

Loading