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

The dependency analyzer extends projects scope by pulling the dependencies into it #405

Closed
wenns opened this issue Jan 26, 2015 · 9 comments
Labels
Milestone

Comments

@wenns
Copy link
Contributor

wenns commented Jan 26, 2015

The bug can be observed when analysing "test/resources/org/sonar/plugins/cxx/include-directories-project". "sonar.sources" is set to "src". Nevertheless, the dependency analyser pulls "include" and "include_snd" into SQ, resulting in files=10 (should be: 1). Ouch...

@wenns wenns added the bug label Jan 26, 2015
wenns added a commit that referenced this issue Jan 27, 2015
…cts scope by pulling the dependencies into it)
@wenns
Copy link
Contributor Author

wenns commented Jan 27, 2015

@Typz: Can you have a look, please? I've tried a fix but earned a NullPointerException. See the branch 'dependency_analyzer_bug'. It also includes a behave test which triggers the bug.

@wenns wenns added this to the M 0.9.3 milestone Jan 27, 2015
@francoisferrand
Copy link
Contributor

I will try to check this before the weekend.

On 27 janv. 2015, at 19:57, Waleri Enns [email protected] wrote:

@Typz: Can you have a look, please? I've tried a fix but earned a NullPointerException. See the branch 'dependency_analyzer_bug'. It also includes a behave test which triggers the bug.


Reply to this email directly or view it on GitHub.

@francoisferrand
Copy link
Contributor

Issue is in DsmSerializer.serializeCell(), where "dependencyIndex.get(cell.getEdge())" can now return null (and thus get us a NullPointerException).

It is easy to fix there, and it makes the test pass; however I am not sure this is sufficient: we may still end up importing the file somehow (since they will get in the serialized DSM: just now without the dependency).

A complete fix would be to "simply" filter the whole packages and files graphs at the beginning of save(), to ensure we don't introduce the extra files in any place [neither in saved deps, feedback cycles, or DSM]... Filtering in addFile() would be much simpler (e.g. one test in line 108), but I am not sure the included files have been indexed yet...

I'll keep looking...

@wenns
Copy link
Contributor Author

wenns commented Jan 28, 2015

Thanks for poking. FYI: dependency analysis runs as part of the Squid sensor, which is after indexing files.

@francoisferrand
Copy link
Contributor

Well that's cool, should allow for a nice and clean fix. I'll try that.

On 28 janv. 2015, at 23:48, Waleri Enns [email protected] wrote:

Thanks for poking. FYI: dependency analysis runs as part of the Squid sensor, which is after indexing files.


Reply to this email directly or view it on GitHub.

francoisferrand added a commit to francoisferrand/sonar-cxx that referenced this issue Jan 30, 2015
Do not include external files in package/file graphs, this causes the dependency analyzer
to extend projects scope by pulling the dependencies into it.
@francoisferrand
Copy link
Contributor

Updated the branch 'dependency_analyzer_bug' with a working patch.
I have an issue with the smoketest though: the multiple include rule is somehow not activated, so extra warning is generated instead...

@wenns
Copy link
Contributor Author

wenns commented Jan 30, 2015

Cool, thanks. Extra warning: will have a look.

@guwirth
Copy link
Collaborator

guwirth commented Apr 10, 2015

@Typz Could you provide a solution by 2015-04-19 for this? If not we remove it from 0.9.3.

@francoisferrand
Copy link
Contributor

The branch was merged by @wenns some times ago, it should be fine I believe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants