update to checkstyle 8.19

[Bananeweizen]

#145

• use library 8.19
• move tabWidth from TreeWalker to Check
• add AnnotationLocation values and defaults

I've verified this version at runtime regarding the AnnotationLocation
changes, but I'm not sure where to verify the tabWidth changes in the
EclipseCS UI.

This is part of #145, but contains only the library upgrade, not the necessary version changes for the EclipseCS release (as Lars typically splits this into 2 commits).

[lkoe]

Please note that the tabWidth setting of the active configuration is used to calculate editor annotations.
As the property moved this might not work anymore.
I'll look the thing up later, if you can't find it.

On version number update, yes I always do this as part of the release process, separate from the other changes.

[lkoe]

eclipse-cs/net.sf.eclipsecs.core/src/net/sf/eclipsecs/core/builder/Auditor.java

Line 515 in 214b6b0

calculatedColumn += mAdditionalConfigData.getTabWidth();

Coming from here:

eclipse-cs/net.sf.eclipsecs.core/src/net/sf/eclipsecs/core/builder/Auditor.java

Line 144 in 214b6b0

ConfigurationReader.AdditionalConfigData additionalData = CheckerFactory

Ultimately originating from here:

eclipse-cs/net.sf.eclipsecs.core/src/net/sf/eclipsecs/core/config/ConfigurationReader.java

Line 165 in 214b6b0

public static AdditionalConfigData getAdditionalConfigData(InputSource in)

[rnveach]

@lkoe

As the property moved this might not work anymore.

Just to confirm, property did not move, it was added to Checker and to all FileSets. Before this change it was only in Checks and TreeWalker. See checkstyle/checkstyle@87a3357#diff-f7f32abc19eaa1b7ce3ac0595324121a .
So all modules besides filters should be allowed to set tabWidth.
Our current preference is that the user set tabWidth in Checker over the old TreeWalker so it is used for the entire config.

[lkoe]

@rnveach
Does it make sense to set tabWidth differently for different modules in a single configuration?
I would expect it do only be defined in a central spot.

In any case having tabWidth configured on potentially multiple places makes it much harder for eclipse-cs to figure out which tabWidth to use.
This calculation is necessary because Checkstyle warning column numbers are different to how the eclipse editor counts offset.

[rnveach]

Does it make sense to set tabWidth differently for different modules in a single configuration?
I would expect it do only be defined in a central spot.

Before our change, all checks could configure tabWidths differently. You could specify it in TreeWalker, and override it in each individual check making them all have different widths. So adding it to more places wasn't that much different and at the time we thought it wasn't a bad idea.

When upgrading sonar plugin, this became a huge contention and long discussions. checkstyle/sonar-checkstyle#204 and checkstyle/sonar-checkstyle#209 .

We were leaning to how you suggest, only 1 place to set tabWidth but @romani deemed we should leave it as it is and discuss it in main repo and not change it now. See checkstyle/sonar-checkstyle#209 (comment) .
Nothing more has taken place on the subject.

This calculation is necessary because Checkstyle warning column numbers are different to how the eclipse editor counts offset.

Can you give me a basic example so we can understand better? How are the counts different from Checkstyle and Eclipse.

[romani]

@Bananeweizen , please try to reference issue number in git commit message, it helps a lot. Commit message better to be "Issue #145: update to checkstyle 8.19"

[romani]

deemed we should leave it as it is and discuss it in main repo and not change it now.

Yes, let's keep it for now as is, as it did not make any problems before, it is ok to keep it. We need to analyse proposed change before making it.

[rnveach]

@romani

BIN -11 MB net.sf.eclipsecs.checkstyle/checkstyle-8.18-all.jar
BIN +23.1 MB net.sf.eclipsecs.checkstyle/checkstyle-8.19-all.jar

Do you know what caused checkstyle all jar to basically double in size in 1 release?
https://github.com/checkstyle/checkstyle/releases

[romani]

size issue was moved to checkstyle/checkstyle#6713

[Bananeweizen]

I've modified the ConfigurationReader to take the tabWidth from the first occurrence at either Checker or TreeWalker now. Please be aware that I could not run the newly added tests locally (due my company firewall restricting the Internet access), so I will have to fix potential test failures from a private machine tomorrow.

[rnveach]

travis:

[ERROR] Cannot resolve project dependencies:
[ERROR] Software being installed: net.sf.eclipsecs.feature.group 8.18.0.qualifier
[ERROR] Missing requirement: net.sf.eclipsecs.feature.group 8.18.0.qualifier requires 'org.eclipse.equinox.p2.iu; net.sf.eclipsecs.checkstyle [8.18.0,8.18.1)' but it could not be found
[ERROR]

[lkoe]

This accidental number update causes the build to fail

[romani]

@Bananeweizen , please find time to finish this PR.

[romani]

This is PR is abandoned ....
upgrade will be done in scope of #167