Skip to content

Commit

Permalink
JS-160 Add fail-safe in case of deserialization error (#4736)
Browse files Browse the repository at this point in the history
  • Loading branch information
quentin-jaquier-sonarsource authored Jun 13, 2024
1 parent 282a8a0 commit c180c01
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,9 @@ public static <T> T from(Node node, Class<T> clazz) {
case UNRECOGNIZED ->
throw new IllegalArgumentException("Unknown node type: " + node.getType() + " at " + node.getLoc());
};
if (!clazz.isInstance(estreeNode)) {
throw new IllegalStateException("Expected " + clazz + " but got " + estreeNode.getClass());
}
return clazz.cast(estreeNode);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -572,4 +572,17 @@ void throw_exception_from_unrecognized_type() {
.isInstanceOf(IllegalArgumentException.class)
.hasMessageStartingWith("Unknown node type: UNRECOGNIZED");
}

@Test
void throw_exception_for_incorrect_cast() {
Node block = Node.newBuilder()
.setType(NodeType.BlockStatementType)
.setBlockStatement(BlockStatement.newBuilder().build())
.build();

assertThatThrownBy(() -> ESTreeFactory.from(block, ESTree.Super.class))
.isInstanceOf(IllegalStateException.class)
.hasMessage("Expected class org.sonar.plugins.javascript.api.estree.ESTree$Super " +
"but got class org.sonar.plugins.javascript.api.estree.ESTree$BlockStatement");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,7 @@ protected void analyzeFile(InputFile file, @Nullable List<String> tsConfigs, @Nu
CacheAnalysis.fromResponse(response.ucfgPaths(), response.cpdTokens()),
file
);
Node responseAst = response.ast();
if (responseAst != null) {
// When we haven't serialized the AST:
// either because no consumer is listening
// or the file extension or AST nodes are unsupported
consumers.accept(new JsFile(file, ESTreeFactory.from(responseAst, ESTree.Program.class)));
}
acceptAstResponse(response, file);
} catch (IOException e) {
LOG.error("Failed to get response while analyzing " + file.uri(), e);
throw e;
Expand All @@ -131,6 +125,21 @@ protected void analyzeFile(InputFile file, @Nullable List<String> tsConfigs, @Nu
}
}

private void acceptAstResponse(BridgeServer.AnalysisResponse response, InputFile file) {
Node responseAst = response.ast();
if (responseAst != null) {
// When we haven't serialized the AST:
// either because no consumer is listening
// or the file extension or AST nodes are unsupported
try {
ESTree.Program program = ESTreeFactory.from(responseAst, ESTree.Program.class);
consumers.accept(new JsFile(file, program));
} catch (Exception e) {
LOG.debug("Failed to deserialize AST for file: {}", file.uri(), e);
}
}
}

private BridgeServer.JsAnalysisRequest getJsAnalysisRequest(
InputFile file,
@Nullable String fileContent,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,51 @@ public void doneAnalysis() {
assertThat(consumer.done).isTrue();
}

@Test
void should_not_invoke_analysis_consumers_when_cannot_deserialize() throws Exception {
var consumer = new JsAnalysisConsumer() {
final List<JsFile> files = new ArrayList<>();
boolean done;

@Override
public void accept(JsFile jsFile) {
files.add(jsFile);
}

@Override
public void doneAnalysis() {
done = true;
}
};

var sensor = new JsTsSensor(
checks(ESLINT_BASED_RULE, "S2260"),
bridgeServerMock,
analysisWithProgram(),
analysisWithWatchProgram(),
new AnalysisConsumers(List.of(consumer))
);

var inputFile = createInputFile(context);
var tsProgram = new TsProgram("1", List.of(inputFile.absolutePath()), List.of(), false, null);
when(bridgeServerMock.createProgram(any())).thenReturn(tsProgram);

Node erroneousNode = Node.newBuilder()
.setType(NodeType.BlockStatementType)
.build();

when(bridgeServerMock.analyzeTypeScript(any())).thenReturn(
new AnalysisResponse(null, List.of(), List.of(), List.of(), new BridgeServer.Metrics(), List.of(), List.of(), erroneousNode)
);

sensor.execute(context);
assertThat(consumer.files).isEmpty();
assertThat(consumer.done).isTrue();

assertThat(logTester.logs(Level.DEBUG))
.contains("Failed to deserialize AST for file: " + inputFile.uri());
}

private JsTsSensor createSensor() {
return new JsTsSensor(
checks(ESLINT_BASED_RULE, "S2260"),
Expand Down

0 comments on commit c180c01

Please sign in to comment.