-
Notifications
You must be signed in to change notification settings - Fork 125
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
SLE-1000: Embed CFamily analyzer #781
Conversation
64ab66d
to
eb23745
Compare
Embed the CFamily analyzer conditionally and provide documentation to the forks. Update the NOTICE.txt to include the note about the non-Open Source analyzer.
6b36aed
to
b41117d
Compare
Move the CDT tests from the SonarQube Cloud axis and remove the dead code or references. Update the CI pipeline in order to parallelize the work IT tasks.
f90d94d
to
ff9e584
Compare
context.setAnalysisProperty(SonarLintUtils.SONARLINT_ANALYSIS_CDT_EXCLUSION_PROPERY, excludedFilesPerUri); | ||
|
||
logger.error("The following C/C++ files were excluded by the CDT integration as no information could be " | ||
+ "accessed from the CDT plug-in. This might happen when the CDT plug-in is not yet ready or when the " |
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.
Are there any Eclipse docs that we can link to help troubleshoot the CDT plug-in? Ideally, an official resource that hints users that building their projects is required in some cases (e.g. CMake). WDYT?
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.
Hi Michael, I will check for that.
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.
@michael-jabbour-sonarsource there is no official documentation page about it but one for the Scanner Discovery against the symbols not found errors and one about setting the include paths for the indexer.
I think it might be better to link to the official documentation as a whole here, WDYT? It contains a lot more information about configuring and tweaking projects ^^
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.
Thanks for sharing the resources! For me the most helpful section was:
- Built-in Settings. CDT will try to detect built-in compiler symbols and include paths running the compiler with special options and parse the output of this special run. Most compilers provide such an option to print built-in include paths and symbols. Built-in settings are implied and do not get passed to compiler during regular compilation.
- Build Output Parser (BOP). Another method that CDT employs is to analyze build output of the regular build with Build Output Parser. Often, include paths are supplied to the compiler with -I options, and macros with -D options and BOP will try to find those in the output. That method relies on verbose build output of your build where all these options are actually printed by make.
This helped me understand that, in some cases (2), CDT inspects the build output to extract the information needed for code highlighting to work (and therefore needs a build to work). This doesn't seem to be related directly to compile_commands.json
generation as I had expected though (for example, in Meson and Makefile, no compilation databases seem to be generated).
What about having a section in our own docs that says that for C and C++, we rely on the CDT plugin to retrieve the build configuration. We can list a few resources to the official plugin, and hint that building may be required to get this to work. We can then point the error message to our docs. WDYT?
(this is only an idea that came to mind, I will let you decide if it is worth it! 😄 )
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.
I reworked this to spawn a notification instead of just an error message in the log. This also now features the link to the official Eclipse CDT documentation while our Docs team is working on changing it for SonarQube for Eclipse.
For the latter, I already reached out to them, and they are going to create a ticket for it. I will work with them together on the phrasing and the content.
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.
Once Docs replied to my mail, I will link the Docs ticket to this Jira ticket ^^
When the analysis is started but CDT cannot provide information about (e.g. because it is not yet ready, ill configured project). In this cases, firstly a NullPointerException was thrown, and secondly the analysis failed. We mitigated that issue by "removing" (not excluding - to not confuse the terms) these files from the analysis and logging an error with enough information. This way users can still analyze files but CFamily analysis will be skipped. A unit test was fixed that was always succeeding but testing an incorrect behavior.
ff9e584
to
485ced1
Compare
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.
Looks ok to me on a high level
its/org.sonarlint.eclipse.its.cdt/src/org/sonarlint/eclipse/its/cdt/CdtIntegrationTest.java
Outdated
Show resolved
Hide resolved
org.sonarlint.eclipse.core/src/org/sonarlint/eclipse/core/internal/jobs/AnalyzeProjectJob.java
Outdated
Show resolved
Hide resolved
Quality Gate passedIssues Measures |
SLE-1000
Embed the CFamily analyzer conditionally and provide documentation to forks.
Update the NOTICE.txt to include the note about the non-Open Source analyzer.