[patch] Include path containing variables treated as invalid

[GoogleCodeExporter]

Using latest delivered or trunk version of cppcheclipse and latest version of 
Cppcheck.

My project is configured to use variables like ${gccIncludeLocation} that is 
contributed by another plug-in. Upon running the cppcheck action, the included 
is traited as invalid, stack trace provided:

java.net.URISyntaxException: Illegal character in opaque part at index 2: 
C:\Program Files\Eclipse\eclipse\plugins\my.plugin\includes
at java.net.URI$Parser.fail(Unknown Source)
at java.net.URI$Parser.checkChars(Unknown Source)
at java.net.URI$Parser.parse(Unknown Source)
at java.net.URI.<init>(Unknown Source)
at 
com.googlecode.cppcheclipse.ui.ToolchainSettings.getIncludes(ToolchainSettings.j
ava:167)
at 
com.googlecode.cppcheclipse.ui.ToolchainSettings.getUserIncludes(ToolchainSettin
gs.java:91)

Patch provided that appears to fix the issue

Cheers
Francois

Original issue reported on code.google.com by rigault....@gmail.com on 14 Oct 2011 at 9:23

[GoogleCodeExporter]

Original comment by rigault....@gmail.com on 15 Oct 2011 at 11:54

Attachments:

• patch.uritopath.txt

[GoogleCodeExporter]

Thanks for your patch, but using IPathVariableManager.resolvePath(IPath path) 
is marked as deprecated. Instead it is recommended to use 
IPathVariableManager.resolveURI(URI uri). But I see that this is no option with 
your placeholders. Maybe you can tell me about the other plugin you are using, 
which uses invalid placeholders for URIs like ${gccIncludeLocation}

Original comment by konra...@gmx.de on 30 Oct 2011 at 10:45

[GoogleCodeExporter]

Also have a look at the Javadoc of IPathVariableManager
...
 A path variable is a pair of non-null elements (name,value) where name is 
 * a case-sensitive string (containing only letters, digits and the underscore
 * character, and not starting with a digit)
...
Within URIs those variables may only appear in the first section and the name 
is not starting with ${...}, but instead something like FOO/include/myheader.h 
which is resolved due to FOO being a variable.

Original comment by konra...@gmx.de on 30 Oct 2011 at 10:52

[GoogleCodeExporter]

Thanks for your comments

${stuff} is the valid syntax to reference a variable (if you go through the 
wizard and choose to add a new path, select a build variable like eclipse_home, 
it will be added as ${eclipse_home} to the project includes. Indeed my variable 
is named gccIncludeLocation.

gccIncludeLocation maps to C:\Program Files\... which is valid for CDT but not 
for cppcheclipse (see below to reproduce without third party plug-ins.

sorry for the use of deprecated function (it was not deprecated back to Eclipse 
3.5.2... a long time ago - I should have use Path.toFile.toURI or similar...). 
Although I'm using a deprecated function, note that the exception did not came 
from the use of the PathVariableManager, but from the line:
> pathUri = new URI(pathString);

where pathString has been *already* resolved using the provided variable. The 
fact that the variable resolves to a path (C:\..) and not to an URI (/C:/...) 
causes the issue. My above patch also failed to interpret variables that map to 
URI, so it's broken as well.

You can reproduce without third party plug-in (here using Eclipse 3.5.2)
- Define a build variable in Preferences > C/C++ > Build Variables > Add > 
(Path) with
    name = foobar
    value = D:\toto
- Add ${foobar} in the C++ includes list of your project. It will be correctly 
interpreted by CDT
- cppcheclipse fails to interpret the variable.

Anyway, redefining my variables so that their value correspond to an URI just 
fixes the problem, ie for foobar above set
    value = /D:/toto

both "styles" are understood correctly by CDT.

Original comment by rigault....@gmail.com on 31 Oct 2011 at 10:52

[GoogleCodeExporter]

> select a build variable like eclipse_home, it will be added as 
${eclipse_home} to the project includes.

if Eclipse is installed within C:\Program Files with the space... it still 
fails to decode the URI:

java.net.URISyntaxException: Illegal character in path at index 11: /C:/Program 
Files/Eclipse-3.5-ODS/eclipse/
at java.net.URI$Parser.fail(Unknown Source)
at java.net.URI$Parser.checkChars(Unknown Source)
at java.net.URI$Parser.parseHierarchical(Unknown Source)
at java.net.URI$Parser.parse(Unknown Source)
at java.net.URI.<init>(Unknown Source)
at 
com.googlecode.cppcheclipse.ui.ToolchainSettings.getIncludes(ToolchainSettings.j
ava:167)

Original comment by rigault....@gmail.com on 31 Oct 2011 at 11:10

[GoogleCodeExporter]

Ok, thanks for your input. Seems that the CDT-Include Paths cannot be easily 
converted into a URI. I will find a workaround for that, or maybe even discard 
the PathVariableManager at all.

Original comment by konra...@gmx.de on 31 Oct 2011 at 11:26

• Changed state: Accepted
• Added labels: Milestone-0.9.8

[GoogleCodeExporter]

This issue was closed by revision r228.

Original comment by konra...@gmx.de on 31 Oct 2011 at 6:59

• Changed state: Fixed

[GoogleCodeExporter]

Hello

Thanks !
the issue is fixed for above examples, but the change introduced a regression -
Testing done:
- include path ${foobar}/toto with ${foobar} = D:\toto
- include path ${foobar}/toto with ${foobar} = D:\to to
- include path ${foobar}/toto with ${foobar} = /D:/toto
- include path ${foobar}/toto with ${foobar} = /D:/to to
- include path (workspace) /${ProjName}/toto

Last case that was passing with previous version is now failing
Since getIncludes is first computing an absolute path to the file and then 
processing variables, the include path is transformed this way:
/${ProjName}/toto 
-> C:\pathtoeclipse\${ProjName}\toto
-> C:\pathtoeclipse${ProjName}\toto (in CdtVariableResolver which interprets \ 
as the escape char

This case is now failing.

Thanks a lot for your work ! I'll open another issue if needed

Francois

Original comment by rigault....@gmail.com on 3 Nov 2011 at 9:29

[GoogleCodeExporter]

Thanks for your test cases. I'll try to fix this issue as well.

Original comment by konra...@gmx.de on 5 Nov 2011 at 9:19

• Changed state: Accepted

[GoogleCodeExporter]

This issue was updated by revision r259.

Original comment by konra...@gmx.de on 10 Sep 2012 at 8:16

[GoogleCodeExporter]

I added your testcases as automated tests. Still missing is the tests for 
relative URLs, Workspace Variables and linked folders. 
I encourage everyone to help.

Original comment by konra...@gmx.de on 10 Sep 2012 at 8:24

[GoogleCodeExporter]

With revision r306 I switched to using CDTs internal mechanism of resolving 
variables. I performed some tests, and all pass. 

Original comment by konra...@gmx.de on 1 Mar 2014 at 8:26

• Changed state: Fixed
• Added labels: Milestone-1.0.0
• Removed labels: Milestone-0.9.8