PicnicSupermarket / error-prone-support

Error Prone extensions: extra bug checkers and a large battery of Refaster rules.
https://error-prone.picnic.tech
MIT License
199 stars 39 forks source link

Can't disable refaster rule using a compiler argument #654

Open vprudnikov opened 1 year ago

vprudnikov commented 1 year ago

Can't disable rule using a compiler argument

Minimal Reproducible Example

Here is my configuration in pom.xml

...
        <error-prone.version>2.19.1</error-prone.version>
        <error-prone-slf4j.version>0.1.18</error-prone-slf4j.version>
        <error-prone-support.version>0.11.1</error-prone-support.version>
...
                <plugin>
                    <groupId>org.apache.maven.plugins</groupId>
                    <artifactId>maven-compiler-plugin</artifactId>
                    <version>${maven-compiler-plugin.version}</version>
                    <configuration>
                        <source>${java.version}</source>
                        <target>${java.version}</target>
                        <encoding>${project.build.sourceEncoding}</encoding>
                        <compilerArgs>
                            <arg>-XDcompilePolicy=simple</arg>
                            <!---DisableWarningsInGeneratedCode because it fails with Lombok-->
                            <!---Disable CheckArgumentWithMessage because we already check arguments with Lombok-->
                            <!---Disable ImmutableMapOf because we prefer using standard Java library even Map.of() looks confusing since it returns an immutable map-->
                            <arg>
                                -Xplugin:ErrorProne
                                -XepDisableWarningsInGeneratedCode
                                -XepOpt:Refaster:NamePattern=^(?!PreconditionsRules\$CheckArgumentWithMessage).*
                                -XepOpt:Refaster:NamePattern=^(?!ImmutableMapRules\$ImmutableMapOf).*
                                -XepOpt:Refaster:NamePattern=^(?!ImmutableMapRules\$ImmutableMapOf1).*
                                -XepOpt:Refaster:NamePattern=^(?!ImmutableMapRules\$ImmutableMapOf2).*
                                -XepOpt:Refaster:NamePattern=^(?!ImmutableMapRules\$ImmutableMapOf3).*
                                -XepOpt:Refaster:NamePattern=^(?!ImmutableMapRules\$ImmutableMapOf4).*
                                -XepOpt:Refaster:NamePattern=^(?!ImmutableMapRules\$ImmutableMapOf5).*
                            </arg>
                        </compilerArgs>
                        <annotationProcessorPaths>
                            <!-- Error Prone. More info: https://errorprone.info/index -->
                            <path>
                                <groupId>com.google.errorprone</groupId>
                                <artifactId>error_prone_core</artifactId>
                                <version>${error-prone.version}</version>
                            </path>
                            <!-- Error Prone additional bug checkers. More info: https://github.com/KengoTODA/errorprone-slf4j -->
                            <path>
                                <groupId>jp.skypencil.errorprone.slf4j</groupId>
                                <artifactId>errorprone-slf4j</artifactId>
                                <version>${error-prone-slf4j.version}</version>
                            </path>
                            <!-- Error Prone additional bug checkers. More info: https://error-prone.picnic.tech -->
                            <path>
                                <groupId>tech.picnic.error-prone-support</groupId>
                                <artifactId>error-prone-contrib</artifactId>
                                <version>${error-prone-support.version}</version>
                            </path>
                            <!-- Error Prone Refaster rules. More info: https://error-prone.picnic.tech -->
                            <path>
                                <groupId>tech.picnic.error-prone-support</groupId>
                                <artifactId>refaster-runner</artifactId>
                                <version>${error-prone-support.version}</version>
                            </path>
                            <path>
                                <groupId>org.projectlombok</groupId>
                                <artifactId>lombok</artifactId>
                                <version>${lombok.version}</version>
                            </path>
                        </annotationProcessorPaths>
                    </configuration>
                </plugin>
Logs ```sh [INFO] /C:/dev/.../MyClass.java:[30,70] [Refaster Rule] PreconditionsRules.CheckArgumentWithMessage: Refactoring opportunity (see https://error-prone.picnic.tech/refasterrules/PreconditionsRules#CheckArgumentWithMessage) Did you mean 'public DescribedPredicate> havingAttribute(checkArgument(@NonNull != @NonNull, @NonNull); String attributeName) {'? ```

Expected behavior

The rule doesn't appear in the compiler output.

Setup

Stephan202 commented 1 year ago

Hey @vprudnikov! Thanks for reaching out. I can see how our documentation is confusing in this respect, but unfortunately only the last -XepOpt:Refaster:NamePattern flag is respected. If you want to disable multiple rules then they'll have to be combined, a bit like this, but then as a negation.

I haven't tested it, but in your case the following may work:

-XepOpt:Refaster:NamePattern=^(?!PreconditionsRules\$CheckArgumentWithMessage)(?!ImmutableMapRules\$ImmutableMapOf).*

(The second one should cover all ImmutableMapOf variants, since it's a prefix match.)

Let us know whether this helps; I'll try to find some time later to clarify the documentation.

vprudnikov commented 1 year ago

@Stephan202 many thanks for a quick answer. I can confirm that the following option works for me: -XepOpt:Refaster:NamePattern=^(?!PreconditionsRules\$CheckArgumentWithMessage)(?!ImmutableMapRules\$ImmutableMapOf\d*).*

Hope this issue will help others in case of confusion.

Stephan202 commented 1 year ago

Thanks for the quick feedback @vprudnikov! I've relabelled this as a documentation issue. My plate is rather full right now, but "one of these days" I'll circle back to this ticket and have a look at better documentation. (And maybe we can even auto-detect or support this case; TBD.)