Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MatchXpath: should support comments #10107

Closed
rnveach opened this issue Jun 11, 2021 · 3 comments · Fixed by #10368
Closed

MatchXpath: should support comments #10107

rnveach opened this issue Jun 11, 2021 · 3 comments · Fixed by #10368
Milestone

Comments

@rnveach
Copy link
Member

rnveach commented Jun 11, 2021

Similar to #7531

$ cat TestClass.java
public class TestClass {
    void method() {
// test
    }
}

$ cat TestConfig.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">

<module name="Checker">
    <property name="charset" value="UTF-8"/>

    <module name="TreeWalker">
<module name="MatchXpath">
  <property name="query" value="//SINGLE_LINE_COMMENT" />
</module>
    </module>
</module>

$ java -jar checkstyle-8.43-all.jar -c TestConfig.xml TestClass.java
Starting audit...
Audit done.

MatchXpath can't flag comments because it is not a comment aware check.

I don't believe it is at all related to PR #9832 since xpath relies on the tokens given to it and checks that aren't comment aware won't receive comments.

@AkMo3
Copy link
Contributor

AkMo3 commented Jul 16, 2021

I am on it !

@strkkk
Copy link
Member

strkkk commented Jul 16, 2021

Just a note - this change can break backwards compatibility.

AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Jul 16, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Jul 17, 2021
@romani romani changed the title MatchXpath: doesn't support comments MatchXpath: should support comments Jul 17, 2021
@romani romani added the bug label Jul 17, 2021
@romani romani added this to the 8.45 milestone Jul 17, 2021
@romani
Copy link
Member

romani commented Jul 17, 2021

fix is merged.
@AkMo3 , thanks a lot for your help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants