Explain reason why Regexp can not be supported by Xpath

[rnveach]

Child of #5777

This issue specifically focuses on Regexp.

[DXTkastb]

I'm on it.

[DXTkastb]

@rnveach This check operates on a plain text file.(java file read as text to obtain any regex pattern)
I don't think AST has any part in this. Passing ast to log() won't make any sense. Please correct me if I am wrong.

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/checks/regexp/RegexpCheck.java

Lines 598 to 613 in 3b3f724

	public void beginTree(DetailAST rootAST) {
	matcher = format.matcher(getFileContents().getText().getFullText());
	matchCount = 0;
	errorCount = 0;
	findMatch();
	}
	/** Recursive method that finds the matches. */
	private void findMatch() {
	final boolean foundMatch = matcher.find();
	if (foundMatch) {
	final FileText text = getFileContents().getText();
	final LineColumn start = text.lineColumn(matcher.start());
	final int startLine = start.getLine();
	final boolean ignore = isIgnore(startLine, text, start);

I am at a dead-end for this check. Also, suggest if any possible changes could be made.

[rnveach]

@romani ping

It looks like this check isn't AST based and shouldn't even be AbstractCheck really. It looks like we will have to create an override to ignore this check from being part of xpath suppression and maybe look to convert it to AbstractFileSetCheck in the future.

[rnveach]

@DXTkastb Please move forward like I described. This check will never support xpath so it should be removed to avoid any confusion with users.

[romani]

Yes, I confirm that this Check is grey area.
It is AbstractCheck based on long history, and it looks like it need to be changed to be based on AbstractFileSetCheck but it will be huge damage for compatibility. We already did same problem with LineLenght it was tough for community to migrate. So no need to migrate Check to new parent class and parent module in config.
It is better to avoid such problem again and just keep it in this list of Currently, filter does not support the following checks at https://checkstyle.org/config_filters.html#SuppressionXpathFilter_Description .

please update this list item to be Regexp (reason is at #7759). reference of issue should be a link to this issue.

this is all that is required to be done in scope of this issue. @DXTkastb , thanks a lot for bringing our attention to this.

[ImmortalRabbit]

Are you sure about that?

please update this list item to be Regexp (reason is at #7759). reference of issue should be a link to this issue.

this is all that is required to be done in scope of this issue. @DXTkastb , thanks a lot for bringing our attention to this.

Because if I do it, the test fails

Incompatible check list should match XpathRegressionTest.INCOMPATIBLE_CHECK_NAMES
missing (1)   : RegexpSinglelineJava
unexpected (1): RegexpSinglelineJava (reason is at #7760)
---
expected      : [CommentsIndentation, AnnotationUseStyle, AnnotationOnSameLine, Regexp, MissingJavadocPackage, TodoComment, MissingCtor, MissingOverride, JavadocMethod, AvoidStaticImport, MissingJavadocMethod, NeedBraces, NoClone, FinalClass, JavadocType, WriteTag, JavadocContentLocation, JavadocStyle, VariableDeclarationUsageDistance, MethodCount, SuppressWarningsHolder, PackageAnnotation, AnnotationLocation, EmptyLineSeparator, IllegalCatch, MissingSwitchDefault, AvoidEscapedUnicodeCharacters, OneTopLevelClass, NoFinalizer, OverloadMethodsDeclarationOrder, NoLineWrap, PackageDeclaration, ImportOrder, CustomImportOrder, UnnecessaryParentheses, AvoidStarImport, MissingJavadocType, ArrayTrailingComma, Indentation, InterfaceIsType, LambdaParameterName, UncommentedMain, InterfaceMemberImpliedModifier, OuterTypeFilename, RegexpSinglelineJava, InvalidJavadocPosition, TrailingComment]
but was       : [CommentsIndentation, AnnotationUseStyle, AnnotationOnSameLine, Regexp, MissingJavadocPackage, TodoComment, MissingCtor, MissingOverride, JavadocMethod, AvoidStaticImport, MissingJavadocMethod, NeedBraces, NoClone, FinalClass, JavadocType, WriteTag, JavadocContentLocation, JavadocStyle, VariableDeclarationUsageDistance, MethodCount, SuppressWarningsHolder, PackageAnnotation, AnnotationLocation, EmptyLineSeparator, IllegalCatch, MissingSwitchDefault, AvoidEscapedUnicodeCharacters, OneTopLevelClass, NoFinalizer, OverloadMethodsDeclarationOrder, NoLineWrap, PackageDeclaration, ImportOrder, CustomImportOrder, UnnecessaryParentheses, AvoidStarImport, MissingJavadocType, ArrayTrailingComma, Indentation, InterfaceIsType, LambdaParameterName, UncommentedMain, InterfaceMemberImpliedModifier, OuterTypeFilename, InvalidJavadocPosition, TrailingComment, RegexpSinglelineJava (reason is at #7760)]

	at com.puppycrawl.tools.checkstyle.internal.XdocsPagesTest.validateListOfSuppressionXpathFilterIncompatibleChecks(XdocsPagesTest.java:673)
	at com.puppycrawl.tools.checkstyle.internal.XdocsPagesTest.validateDescriptionSection(XdocsPagesTest.java:665)
	at com.puppycrawl.tools.checkstyle.internal.XdocsPagesTest.validateCheckSection(XdocsPagesTest.java:585)
	at com.puppycrawl.tools.checkstyle.internal.XdocsPagesTest.testAllCheckSections(XdocsPagesTest.java:509)

Actually, it fails right here:

assertWithMessage(
            "Incompatible check list should match XpathRegressionTest.INCOMPATIBLE_CHECK_NAMES")
            .that(getListById(subSection, "SuppressionXpathFilter_IncompatibleChecks"))
            .isEqualTo(XpathRegressionTest.INCOMPATIBLE_CHECK_NAMES);

I could change XpathRegressionTest.INCOMPATIBLE_CHECK_NAMES but it's used in XpathRegressionTest and it fails if I change it. Also, we may try to add the bottom of list small paragraph that says Reason for 'Regexp' can be found here #7759) or something else.

In my case, result will be like that: