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

Issues that don't report a filename are not handled correctly #1489

Closed
AbbasNS opened this issue Jun 22, 2020 · 3 comments · Fixed by #1523
Closed

Issues that don't report a filename are not handled correctly #1489

AbbasNS opened this issue Jun 22, 2020 · 3 comments · Fixed by #1523
Assignees
Labels
Milestone

Comments

@AbbasNS
Copy link
Contributor

AbbasNS commented Jun 22, 2020

Description

Some of the C and C++ rules can be triggered without a specific filename. For example, S3744 will be triggered if when a macro is defined multiples times with different values as a compiler option. This rule will not have an associated filename since the issue doesn't exist in a specific location of the file.

When it happens, the subprocess.exe finishes successfully, however it returns an issue without filename, which causes SLVS to crash.

Repro steps

Create a C++ project and define a macro to two different values using additional options:

image
When analyzing the file you will get a crash while reading the analysis results:
image

Expected behavior

Handle rules without an associated file. Maybe display them a warning without mapping them to a specific location

Actual behavior

No issues are reported for the file (it's also possible no issues will be reported for the entire project, depending on which rule was triggered. See explanation from @abbas-sabra-sonarsource below.).
A message is logged in the SonarLint output window.

Known workarounds

Disable the rule causing the issue from the setting file:
image

@AbbasNS AbbasNS changed the title Fix crash due to issues without filename Fix crash due to issues without a filename Jun 22, 2020
@rita-gorokhod
Copy link
Contributor

rita-gorokhod commented Jun 22, 2020

Another reproduction scenario: Define a macro using additional options to a non-portable file path, which will trigger S3806, such as /FI C:\projects\foo.h

Expected: No crash.
Actual: Same crash as above, "non-portable path to file '"C\Projects\foo.h"'; specified path differs in case from file name on disk".

@duncanp-sonar
Copy link
Contributor

@abbas-sabra-sonarsource thanks for creating the issues.
I've updated the Actual behaviour section to try to describe the impact the user experience. Could you check that it's accurate please?
Also, is it a regression or is it an issue that has been around for some time?

@duncanp-sonar duncanp-sonar changed the title Fix crash due to issues without a filename Analysis fails for issues that don't report a filename Jun 23, 2020
@AbbasNS
Copy link
Contributor Author

AbbasNS commented Jun 23, 2020

@duncanp-sonar, It isn't that analysis fails, the analysis succeeds and returns meaningful issues. the problem happens when we have issues(in this case "code smell") with no specific location(filename). For example, we can find code smell on macro/code defined through additional options => they will not have a real location in the file. We should simply check if the filename is empty before calling GetFullPath.

Q: does it depend on whether the current file being analysed uses the invalid macro?
No.
It isn't an invalid macro. It is valid, It is just a code smell to override the value of a macro in the options. SLVS should either skip these type of issues with no filenames and display the rest, or find a way to display them without a specific location.

Q: does this mean that no issues are reported for any file in the project?
It depends if the entire project is using the same options. or if the option is specified for one file. usually, compilation options are specified per project, not per file.

by the way, there are other kinds of issues that usually don't have a filename. For example when one of our rule input is badly configured we raise an issue with no filename.

Q: Also, is it a regression or is it an issue that has been around for some time?
I don't think this was recently introduced.

Let me know if further clarification is needed.

@duncanp-sonar duncanp-sonar changed the title Analysis fails for issues that don't report a filename Issues that don't report a filename are not handled correctly Jun 23, 2020
@rita-gorokhod rita-gorokhod added this to the CFamily streaming milestone Jul 1, 2020
@rita-gorokhod rita-gorokhod added this to To do in CFamily streaming via automation Jul 1, 2020
@rita-gorokhod rita-gorokhod moved this from To do to In progress in CFamily streaming Jul 1, 2020
@rita-gorokhod rita-gorokhod self-assigned this Jul 1, 2020
@rita-gorokhod rita-gorokhod linked a pull request Jul 2, 2020 that will close this issue
@rita-gorokhod rita-gorokhod moved this from In progress to Review in progress in CFamily streaming Jul 2, 2020
@rita-gorokhod rita-gorokhod moved this from Review in progress to Reviewer approved in CFamily streaming Jul 2, 2020
rita-gorokhod added a commit that referenced this issue Jul 2, 2020
@rita-gorokhod rita-gorokhod moved this from Reviewer approved to Done in CFamily streaming Jul 2, 2020
duncanp-sonar pushed a commit that referenced this issue Jul 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants