-
Notifications
You must be signed in to change notification settings - Fork 181
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-163 New Java API to process files during the analysis #4706
Conversation
...in/plugins/consumer-plugin/src/main/java/org/sonar/samples/javascript/consumer/Consumer.java
Show resolved
Hide resolved
Quality Gate passedIssues Measures |
} | ||
|
||
protected boolean isJavaScript(InputFile file) { | ||
return inputFileLanguage(file).equals(JavaScriptLanguage.KEY); | ||
} | ||
|
||
abstract void analyzeFiles(List<InputFile> inputFiles, List<String> tsConfigs) throws IOException; | ||
|
||
protected void analyzeFile(InputFile file, @Nullable List<String> tsConfigs, @Nullable TsProgram tsProgram) throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice job of refactoring duplicate code into this!
@Override | ||
public void doneAnalysis() { | ||
consumers.forEach(JsAnalysisConsumer::doneAnalysis); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doneAnalysis()
is actually never used for the consumerS, only for each consumer individually?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this IT runs both the JS and Consumer plugins and validates that the consumer plugin runs execute()
if and only if once the JS plugin is done processing all the files? Is that correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it is asserting that execute
is invoked only after the analysis. The order of execution of sensors is implemented with DependsUpon/@DependedUpon
annotation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments and remarks, otherwise, LGTM!
Fixes JS-163