Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JS-160 Add fail-safe in case of deserialization error #4736

Merged
merged 1 commit into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading