Fix #14116 (cmdline: Improve IDE integration with --file-filter=+ option)

[danmar]

No description provided.

[danmar]

@firewave I am particularly interested in feedback from you. :-)

We want that customers are able to reuse the same foo.cppcheck file or whatever both on the command line and in the IDE. We don't want that each developer manually has to configure various IDE plugin options.

[firewave]

This is essentially #6754 with an added CLI option. I prefer the previous version as it is cleaner.

[firewave]

And this needs Python tests which actually test this behavior.

[danmar]

This is essentially #6754 with an added CLI option. I prefer the previous version as it is cleaner.

I don't have a strong opinion. both --project-settings and --file-filter=+ are fine for me.

[danmar]

With --file-filter=+ the user has to use 1 more option. but it's not significant to me.

[firewave]

With --file-filter=+ the user has to use 1 more option. but it's not significant to me.

I implemented it that way so an existing --project= can stay in place. So if you have a hard-code options you can just append --file-filter=+ <file>.

With --project-settings you would not be able to override any existing --project. --project-settings also feels a bit misleading without reading the configuration.

And the --file-filter approach would also allow for simpler documentation ("given files on CLI will be treated as filters"). The documentation added in this PR feels a bit much.

[danmar]

I implemented it that way so an existing --project= can stay in place. So if you have a hard-code options you can just append --file-filter=+ .

Imho the problem is that it's very often <file> that is hardcoded.
So user will need to provide both --project and --file-filter=+ now. But anyway I don't care.. I see no real user problem with that.

[danmar]

And this needs Python tests which actually test this behavior.

Imho the existing --file-filter python tests tests this behavior. We have tested that the settings are set properly..

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[firewave]

Imho the problem is that it's very often <file> that is hardcoded.
So user will need to provide both --project and --file-filter=+ now. But anyway I don't care.. I see no real user problem with that

That depends on the IDE implementation. In CLion the project location is not available so it can only provide the filename.

I know other IDE integrations provide the project and people assumed that by adding the filename they could filter it. But what would have happened in that case that it was analyzing all of the project and the specified file. Me disallowing a project and individual files being used flushed out such a case in the forum (no link handy). How this was actually usable in terms of performance is baffling.

Thinking a bit... it could have been the other way around that it was just the file and people thought adding --project would provide the configuration. Well, that result would have been the same.

I think this is the most flexible and clear approach (pending feedback). I will give it a spin in CLion tomorrow.

[danmar]

That depends on the IDE implementation. In CLion the project location is not available so it can only provide the filename.

I do not understand this really. so as I understand it, the clion plugin checks a temporary file.. ?
let's say the file includes some headers that are located in your project. do clion auomatically create temporary files for those also or is it possible to provide some -I that points at the project?

I will give it a spin in CLion tomorrow.

thanks!!

[firewave]

I do not understand this really. so as I understand it, the clion plugin checks a temporary file.. ?

Yes. That is done because CLion has a virtual filesystem and the changes might not have been applied to the actual filesystem yet. This way it is ensured it is using the current content. It also avoid the file being changed while it is being read for the analysis. IIRC there would access issues in the past without doing this.

I am not sure that is the proper way to do it but it works fine.

let's say the file includes some headers that are located in your project. do clion auomatically create temporary files for those also or is it possible to provide some -I that points at the project?

That is not necessary because in the IDE you only change one file at a time and that is the only file you want to analyze for.

I will give it a spin in CLion tomorrow.

thanks!!

The new approach does not work with CLion because of the temporary file:

cppcheck: error: could not find any files matching the filter:/tmp/ER291l9z_utils.cpp

To get that to work you would require some mapping.

The failing command is
cppcheck --project=proj/cc.json --file-filter=+ /tmp/ER291l9z_utils.cpp

And it would need to be something like (this the first thing I could think of)
cppcheck --project=proj/cc.json --file-filter=+ --file-redirect=/tmp/ER291l9z_utils.cpp proj/lib/utils.cpp

--file-redirect= would be limited to being only specified once with only a single input file parameter. That would also solve the issue that the output currently show the name of the temporary file.

But I have the feeling that it could get quite messy and I would not even try to prototype that until #6379 has been merged.

[danmar]

And it would need to be something like (this the first thing I could think of)

I understand. I am not sure I like this but I do not have better suggestions directly :-(
I do want cppcheck to integrate fine with CLion also.

But I have the feeling that it could get quite messy and I would not even try to prototype that until #6379 has been merged.

me too.. I fear this got quite complex.. I suggest that we leave that for later. And merge this PR as is.

[danmar]

That is not necessary because in the IDE you only change one file at a time and that is the only file you want to analyze for.

It's not 100% true.

If you check C++ code and then only have the .cpp file but not the header it is likely that the checking will be very limited because the function scopes are not created properly without the declarations..

Example code:

void Fred::foo(int*p) {
    *p = 123;
    if (p != nullptr) {}
}

[danmar]

I am very late about this. but I will talk about cppcheck integration at the cppcon in less than 2 weeks. And it would be nice to have some option that allows --project to be used in IDEs.
At the same time.. I don't want to quickly prematurely throw in 1 option now.. and then later discover we need completely other options to make clion work..

[firewave]

I suggest that we leave that for later. And merge this PR as is.

That's fine with me. The additional is perfectly fine and a nice enhancement. I will file a ticket with the CLion case and look into it when the interfaces have been cleaned up (still a couple of PRs which need to go in first).

[firewave]

Maybe --file-input= instead. Then you could also use - and specify a filename to use but could pipe the content in via stdin. That way you would not need to write any temporary file at all. Not sure if that would be less messy.