FinalClassCheck: 'extractQualifiedName' omits 2nd ident of package name

[rnveach]

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/checks/design/FinalClassCheck.java

Line 95 in 90f20e0

packageName = extractQualifiedName(ast);

When debugging tests like testFinalClass, extractQualifiedName returns com.tools.checkstyle.checks.design.finalclass when the package is clearly defined as com.puppycrawl.tools.checkstyle.checks.design.finalclass (puppycrawl is missing).

Looking at the code, when it gets to the top node of the package name, it only pulls in the first IDENT and not them both.

I am not sure if this bug presents itself in the CLI somehow as I am not that familiar with the check.
I found this bug by duplicating the same code in my own custom check.

[rnveach]

@romani Don't we have FullIdent(https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/api/FullIdent.java) class to be used in this type of situation?
Should this method just be removed and we use FullIdent instead? I assume it doesn't have this same bug. Confirmed FullIdent works correctly.

[romani]

I am ok to re-use FullIdent, but one day it will not be API :) class.

[rnveach]

I'm not sure what you mean, API versus util doesn't matter for checks.

[romani]

src/main/java/com/puppycrawl/tools/checkstyle/api/FullIdent.java

Check is ok to reuse this class at any package.

[rnveach]

yes, I know you meant the API package.
But I don't understand from your 1st comment why importing an API class versus a non-API class matters for a check. If FullIdent moves to util package, it won't hurt this check or any check using it.

[rnveach]

If you are referring to my custom check, anyone outside checkstyle can use utils if that is where you plan to move FullIdent to. It is used by so many checks that the constructor can't be private/protected.

[romani]

it was just side note(off topic) for future of FullIdent class.

[rnveach]

Fix was merged