picnicsupermarket / error-prone-support Goto Github PK
View Code? Open in Web Editor NEWError Prone extensions: extra bug checkers and a large battery of Refaster rules.
Home Page: https://error-prone.picnic.tech
License: MIT License
Error Prone extensions: extra bug checkers and a large battery of Refaster rules.
Home Page: https://error-prone.picnic.tech
License: MIT License
I couldn't find any documentation on how to write and apply your own Refaster rules. I've tried adding and enabling your Refaster rule compiler to the build like this:
dependencies {
annotationProcessor("tech.picnic.error-prone-support:refaster-compiler:$errorProneSupportVersion")
// ...
}
tasks.withType<JavaCompile>().configureEach {
options.compilerArgs.add("-Xplugin:RefasterRuleCompiler")
// ...
}
Then I get my regular Java classes with @BeforeTemplate
and @AfterTemplate
compiled into .refaster
files inside build/classes
.
Unfortunately, even though I have the Refaster runner dependency as well, I cannot see any of my custom rules kicking in. I get all the error-prone-support checks tough.
Am I missing something here?
When I look at the documentation of a bug pattern, I can see what the pattern is / wants to enforce, but not why that is better.
Example: https://error-prone.picnic.tech/bugpatterns/AmbiguousJsonCreator/ - I understand that "JsonCreator.Mode should be set for single-argument creators", but I don't know why.
Add a section to the documentation of each bug pattern that explains why it's better to follow the rule. Compare with Sonar rules which usually explain the reasoning behind themselves quite well: https://rules.sonarsource.com/java
This is based on #428 (comment).
Let's rewrite expressions of checkNotNull
and checkArgument
with a null check to a requireNonNull
equivalent.
There are multiple overloads for both methods that we should consider.
I would like to rewrite the following code:
checkNotNull(annotation);
checkNotNull(annotation, "Annotation must be present");
checkArgument(uri.getHost() != null);
checkArgument(uri.getHost() != null, "Host should be present");
to:
requireNonNull(annotation);
requireNonNull(annotation, "Annotation must be present");
requireNonNull(uri.getHost());
requireNonNull(uri.getHost(), "Host should be present");
Question:
What do we want with the checkNotNull
? Do we prefer requireNonNull
in all cases because it is in the JDK?
Answer:
Discussed and updated this ticket. For context, see thread below.
An ImmutableMap.Builder
has two terminal methods which accomplish the same thing: build
or buildOrThrow
.
The buildOrThrow
method should be preferred, because the documentation of build
specifies:
Prefer the equivalent method {@link #buildOrThrow()} to make it explicit that the method will throw an exception if there are duplicate keys. The {@code build()} method will soon be deprecated.
I would like to rewrite the following code:
ImmutableMap.Builder<K, V> builder = ImmutableMap.builder();
builder.build();
to:
ImmutableMap.Builder<K, V> builder = ImmutableMap.builder();
builder.buildOrThrow();
As @Stephan202 mentioned in #364, we can create a Refaster rule that rewrites the following:
requireNonNullElse(map.get(k), defaultValue)
-> map.getOrDefault(k, defaultValue)
.
I would like to rewrite the following code:
Map<Integer, Integer> map = Map.of(1, 2);
Integer integer = requireNonNullElse(map.get(1), 3);
to:
Map<Integer, Integer> map = Map.of(1, 2);
Integer integer = map.getOrDefault(1, 3);
We prefer to use AssertJ for our assertions. So we could use a Refaster rule that automatically prefers this over the assertions library built into JUnit!
I would like to rewrite the following code:
org.junit.jupiter.api.Assertions.assertThrows(
RuntimeException.class,
() -> {
throw new RuntimeException("bar");
});
to:
assertThatThrownBy(() -> {
throw new RuntimeException("bar");
})
.isInstanceOf(RuntimeException.class):
When verifying that mocks didn't have interactions in tests sometimes many calls to Mockito.verifyNoInteractions()
are used
verifyNoInteractions(mock1);
verifyNoInteractions(mock2);
which unnecessarily pollutes the code, because this method actually accepts varargs.
This can be simplified to just:
verifyNoInteractions(mock1, mock2);
We should rewrite multiple calls to verifyNoInteractions
to one call.
I would like to rewrite the following code:
verifyNoInteractions(mock1);
verifyNoInteractions(mock2);
...
verifyNoInteractions(mockN);
to:
verifyNoInteractions(mock1, mock2, ..., mockN);
Tricky parts in my opinion are:
Similar to
but with
java.lang.AssertionError: java.lang.IllegalArgumentException: Cannot combine scanner suppliers with different implementations of 'MissingRefasterAnnotation': tech.picnic.errorprone.bugpatterns.MissingRefasterAnnotation, com.google.errorprone.bugpatterns.MissingRefasterAnnotation
instead of StringCaseLocaleUsage
when upgrading EP from 2.19.1 to 2.20.0.
package org.example;
import lombok.Data;
@Data
public class AnyClass {
private String anything;
}
A full example is available at https://github.com/pacbeckh/error-prone-lombok-issue, run mvn clean install
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.11.0:compile (default-compile) on project errorproneissue: Compilation failure
[ERROR] /<redacted>/error-prone-lombok-issue/src/main/java/org/example/AnyClass.java:[5,1] An unhandled exception was thrown by the Error Prone static analysis plugin.
[ERROR] Please report this at https://github.com/google/error-prone/issues/new and include the following:
[ERROR]
[ERROR] error-prone version: 2.20.0
[ERROR] BugPattern: DirectReturn
[ERROR] Stack Trace:
[ERROR] java.lang.IllegalArgumentException: Replacement{range=[43..48), replaceWith=} conflicts with existing replacement Replacement{range=[43..48), replaceWith=return @Data;}
[ERROR] at com.google.errorprone.fixes.Replacements.add(Replacements.java:166)
[ERROR] at com.google.errorprone.fixes.SuggestedFix.getReplacements(SuggestedFix.java:93)
[ERROR] at com.google.errorprone.fixes.AppliedFix$Applier.apply(AppliedFix.java:69)
[ERROR] at java.base/java.util.HashMap.computeIfAbsent(HashMap.java:1134)
[ERROR] at com.google.errorprone.JavacErrorDescriptionListener.lambda$new$1(JavacErrorDescriptionListener.java:94)
[ERROR] at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195)
[ERROR] at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:177)
[ERROR] at java.base/java.util.Collections$2.tryAdvance(Collections.java:4747)
[ERROR] at java.base/java.util.Collections$2.forEachRemaining(Collections.java:4755)
[ERROR] at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
[ERROR] at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
[ERROR] at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913)
[ERROR] at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
[ERROR] at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578)
[ERROR] at com.google.errorprone.JavacErrorDescriptionListener.onDescribed(JavacErrorDescriptionListener.java:107)
[ERROR] at com.google.errorprone.ErrorProneAnalyzer.lambda$finished$1(ErrorProneAnalyzer.java:141)
[ERROR] at com.google.errorprone.VisitorState.reportMatch(VisitorState.java:301)
[ERROR] at com.google.errorprone.scanner.Scanner.reportMatch(Scanner.java:127)
[ERROR] at com.google.errorprone.scanner.ErrorProneScanner.processMatchers(ErrorProneScanner.java:448)
[ERROR] at com.google.errorprone.scanner.ErrorProneScanner.visitBlock(ErrorProneScanner.java:519)
[ERROR] at com.google.errorprone.scanner.ErrorProneScanner.visitBlock(ErrorProneScanner.java:150)
[ERROR] at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCBlock.accept(JCTree.java:1032)
[ERROR] at jdk.compiler/com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
[ERROR] at com.google.errorprone.scanner.Scanner.scan(Scanner.java:74)
[ERROR] at com.google.errorprone.scanner.Scanner.scan(Scanner.java:48)
[ERROR] at jdk.compiler/com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:90)
[ERROR] at jdk.compiler/com.sun.source.util.TreeScanner.visitMethod(TreeScanner.java:206)
[ERROR] at com.google.errorprone.scanner.ErrorProneScanner.visitMethod(ErrorProneScanner.java:740)
[ERROR] at com.google.errorprone.scanner.ErrorProneScanner.visitMethod(ErrorProneScanner.java:150)
[ERROR] at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCMethodDecl.accept(JCTree.java:898)
[ERROR] at jdk.compiler/com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
[ERROR] at com.google.errorprone.scanner.Scanner.scan(Scanner.java:74)
[ERROR] at com.google.errorprone.scanner.Scanner.scan(Scanner.java:48)
[ERROR] at jdk.compiler/com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:90)
[ERROR] at jdk.compiler/com.sun.source.util.TreeScanner.scan(TreeScanner.java:105)
[ERROR] at jdk.compiler/com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:113)
[ERROR] at jdk.compiler/com.sun.source.util.TreeScanner.visitClass(TreeScanner.java:187)
[ERROR] at com.google.errorprone.scanner.ErrorProneScanner.visitClass(ErrorProneScanner.java:548)
[ERROR] at com.google.errorprone.scanner.ErrorProneScanner.visitClass(ErrorProneScanner.java:150)
[ERROR] at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCClassDecl.accept(JCTree.java:808)
[ERROR] at jdk.compiler/com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:82)
[ERROR] at com.google.errorprone.scanner.Scanner.scan(Scanner.java:74)
[ERROR] at com.google.errorprone.scanner.Scanner.scan(Scanner.java:48)
[ERROR] at jdk.compiler/com.sun.source.util.TreeScanner.scan(TreeScanner.java:105)
[ERROR] at jdk.compiler/com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:113)
[ERROR] at jdk.compiler/com.sun.source.util.TreeScanner.visitCompilationUnit(TreeScanner.java:144)
[ERROR] at com.google.errorprone.scanner.ErrorProneScanner.visitCompilationUnit(ErrorProneScanner.java:560)
[ERROR] at com.google.errorprone.scanner.ErrorProneScanner.visitCompilationUnit(ErrorProneScanner.java:150)
[ERROR] at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCCompilationUnit.accept(JCTree.java:591)
[ERROR] at jdk.compiler/com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:56)
[ERROR] at com.google.errorprone.scanner.Scanner.scan(Scanner.java:58)
[ERROR] at com.google.errorprone.scanner.ErrorProneScannerTransformer.apply(ErrorProneScannerTransformer.java:43)
[ERROR] at com.google.errorprone.ErrorProneAnalyzer.finished(ErrorProneAnalyzer.java:156)
[ERROR] at jdk.compiler/com.sun.tools.javac.api.MultiTaskListener.finished(MultiTaskListener.java:132)
[ERROR] at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1418)
[ERROR] at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1365)
[ERROR] at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:960)
[ERROR] at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.lambda$doCall$0(JavacTaskImpl.java:104)
[ERROR] at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.handleExceptions(JavacTaskImpl.java:147)
[ERROR] at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.doCall(JavacTaskImpl.java:100)
[ERROR] at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.call(JavacTaskImpl.java:94)
[ERROR] at org.codehaus.plexus.compiler.javac.JavaxToolsCompiler.compileInProcess(JavaxToolsCompiler.java:136)
[ERROR] at org.codehaus.plexus.compiler.javac.JavacCompiler.performCompile(JavacCompiler.java:183)
[ERROR] at org.apache.maven.plugin.compiler.AbstractCompilerMojo.execute(AbstractCompilerMojo.java:1140)
[ERROR] at org.apache.maven.plugin.compiler.CompilerMojo.execute(CompilerMojo.java:193)
[ERROR] at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:126)
[ERROR] at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2(MojoExecutor.java:328)
[ERROR] at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute(MojoExecutor.java:316)
[ERROR] at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:212)
[ERROR] at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:174)
[ERROR] at org.apache.maven.lifecycle.internal.MojoExecutor.access$000(MojoExecutor.java:75)
[ERROR] at org.apache.maven.lifecycle.internal.MojoExecutor$1.run(MojoExecutor.java:162)
[ERROR] at org.apache.maven.plugin.DefaultMojosExecutionStrategy.execute(DefaultMojosExecutionStrategy.java:39)
[ERROR] at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:159)
[ERROR] at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:105)
[ERROR] at org.apache.maven.lifecycle.internal.builder.multithreaded.MultiThreadedBuilder$1.call(MultiThreadedBuilder.java:193)
[ERROR] at org.apache.maven.lifecycle.internal.builder.multithreaded.MultiThreadedBuilder$1.call(MultiThreadedBuilder.java:180)
[ERROR] at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
[ERROR] at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
[ERROR] at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
[ERROR] at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
[ERROR] at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
[ERROR] at java.base/java.lang.Thread.run(Thread.java:829)
[ERROR]
I would expect it not to crash.
MacOS Ventura
.11.0.13
).2.20.0
.0.12.0
.A workaround, (which I guess should also be the correct way to work) is to add -XepDisableWarningsInGeneratedCode
to the args, so you get:
<arg>-Xplugin:ErrorProne -XepDisableWarningsInGeneratedCode</arg>
and a lombok.config file with at least the following:
lombok.addLombokGeneratedAnnotation = true
I would like to make some suggestions required for my team by changing the default severity level for them.
It would be nice to provide a compiler argument that changes the severity for one rule, for example:
-XepOpt:Refaster:StreamRules:StreamMapFirst:ERROR
For every Refaster Rule collection we have a corresponding *Test{Input,Output}.java
file where we test the Refaster rules.
These test classes implement the RefasterRuleCollectionTestCase
interface that contains the elidedTypesAndStaticImports
method. This method exists to ensure that the same import statements are (still) required in the *TestOutput
file. Without this, the import(s) could become unused due to the patching of Refaster rules (and therefore removed when running the formatter).
We highly value consistency and therefore always sort the arguments that are passed to the ImmutableSet#of
(see example below).
To enforce this, I propose to make changes to the RefasterRuleCollection
class and verify the sorting of these arguments there.
If the sorting is incorrect, it should suggest the correct order as fix.
I would like to rewrite the following code:
@Override
public ImmutableSet<?> elidedTypesAndStaticImports() {
return ImmutableSet.of(Lists.class, Iterables.class);
}
to:
@Override
public ImmutableSet<?> elidedTypesAndStaticImports() {
return ImmutableSet.of(Iterables.class, Lists.class);
}
This check is rather specific to Error Prone Support itself.
The content of the ImmutableSet
can be basically everything, that might make the sorting a bit challenging?
Maybe we should go one step further and ensure that the expression of the elidedTypesAndStaticImports
is always return ImmutableSet.of(...);
?
This request can be picked up by anyone willing to work on it.
We tried upgrading to com.google.errorprone:error_prone_core:2.19.0
but it brakes compilation suggesting some stuff from Picnic.
Running ./gradlew compileJava
breaks with
IllegalArgumentException: Cannot combine scanner suppliers with different implementations of 'StringCaseLocaleUsage': tech.picnic.errorprone.bugpatterns.StringCaseLocaleUsage, com.google.errorprone.bugpatterns.StringCaseLocaleUsage
An exception has occurred in the compiler (19.0.2). Please file a bug against the Java compiler via the Java bug reporting page (http://bugreport.java.com) after checking the Bug Database (http://bugs.java.com) for duplicates. Include your program, the following diagnostic, and the parameters passed to the Java compiler in your report. Thank you.
java.lang.AssertionError: java.lang.IllegalArgumentException: Cannot combine scanner suppliers with different implementations of 'StringCaseLocaleUsage': tech.picnic.errorprone.bugpatterns.StringCaseLocaleUsage, com.google.errorprone.bugpatterns.StringCaseLocaleUsage
at com.google.errorprone.ErrorProneAnalyzer.lambda$scansPlugins$0(ErrorProneAnalyzer.java:85)
at com.google.common.base.Suppliers$NonSerializableMemoizingSupplier.get(Suppliers.java:183)
at com.google.errorprone.ErrorProneAnalyzer.finished(ErrorProneAnalyzer.java:156)
at jdk.compiler/com.sun.tools.javac.api.MultiTaskListener.finished(MultiTaskListener.java:132)
at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1394)
at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1341)
at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:933)
at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.lambda$doCall$0(JavacTaskImpl.java:104)
at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.invocationHelper(JavacTaskImpl.java:152)
at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.doCall(JavacTaskImpl.java:100)
at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.call(JavacTaskImpl.java:94)
at org.gradle.internal.compiler.java.IncrementalCompileTask.call(IncrementalCompileTask.java:92)
at org.gradle.api.internal.tasks.compile.AnnotationProcessingCompileTask.call(AnnotationProcessingCompileTask.java:94)
at org.gradle.api.internal.tasks.compile.ResourceCleaningCompilationTask.call(ResourceCleaningCompilationTask.java:57)
at org.gradle.api.internal.tasks.compile.JdkJavaCompiler.execute(JdkJavaCompiler.java:55)
at org.gradle.api.internal.tasks.compile.JdkJavaCompiler.execute(JdkJavaCompiler.java:39)
at org.gradle.api.internal.tasks.compile.daemon.AbstractIsolatedCompilerWorkerExecutor$CompilerWorkAction.execute(AbstractIsolatedCompilerWorkerExecutor.java:78)
at org.gradle.workers.internal.DefaultWorkerServer.execute(DefaultWorkerServer.java:63)
at org.gradle.workers.internal.AbstractClassLoaderWorker$1.create(AbstractClassLoaderWorker.java:54)
at org.gradle.workers.internal.AbstractClassLoaderWorker$1.create(AbstractClassLoaderWorker.java:48)
at org.gradle.internal.classloader.ClassLoaderUtils.executeInClassloader(ClassLoaderUtils.java:100)
at org.gradle.workers.internal.AbstractClassLoaderWorker.executeInClassLoader(AbstractClassLoaderWorker.java:48)
at org.gradle.workers.internal.FlatClassLoaderWorker.run(FlatClassLoaderWorker.java:32)
at org.gradle.workers.internal.FlatClassLoaderWorker.run(FlatClassLoaderWorker.java:22)
at org.gradle.workers.internal.WorkerDaemonServer.run(WorkerDaemonServer.java:96)
at org.gradle.workers.internal.WorkerDaemonServer.run(WorkerDaemonServer.java:65)
at org.gradle.process.internal.worker.request.WorkerAction$1.call(WorkerAction.java:138)
at org.gradle.process.internal.worker.child.WorkerLogEventListener.withWorkerLoggingProtocol(WorkerLogEventListener.java:41)
at org.gradle.process.internal.worker.request.WorkerAction.run(WorkerAction.java:135)
at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
at java.base/java.lang.reflect.Method.invoke(Method.java:578)
at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:182)
at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:164)
at org.gradle.internal.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:414)
at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64)
at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:49)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
at java.base/java.lang.Thread.run(Thread.java:1589)
Caused by: java.lang.IllegalArgumentException: Cannot combine scanner suppliers with different implementations of 'StringCaseLocaleUsage': tech.picnic.errorprone.bugpatterns.StringCaseLocaleUsage, com.google.errorprone.bugpatterns.StringCaseLocaleUsage
at com.google.errorprone.scanner.ScannerSupplier.lambda$plus$11(ScannerSupplier.java:255)
at com.google.common.collect.RegularImmutableBiMap.forEach(RegularImmutableBiMap.java:166)
at com.google.errorprone.scanner.ScannerSupplier.plus(ScannerSupplier.java:249)
at com.google.errorprone.ErrorPronePlugins.loadPlugins(ErrorPronePlugins.java:51)
at com.google.errorprone.ErrorProneAnalyzer.lambda$scansPlugins$0(ErrorProneAnalyzer.java:78)
... 40 more
I hope both could work seamlessly
Gradle 8.1.1
Build time: 2023-04-21 12:31:26 UTC
Revision: 1cf537a851c635c364a4214885f8b9798051175b
Kotlin: 1.8.10
Groovy: 3.0.15
Ant: Apache Ant(TM) version 1.10.11 compiled on July 10 2021
JVM: 19.0.2 (Oracle Corporation 19.0.2+7-44)
OS: Mac OS X 13.3.1 aarch64
2.19.0
).0.10.0
).It seems that the StringCaseLocaleUsage has been added by the Error Prone in 2.19.0.
Given below class
import com.fasterxml.jackson.annotation.JsonProperty;
import lombok.Data;
@Data
public class ErrorProneTestObject {
@JsonProperty("event")
private String event;
}
we get unclear recommendations saying that we might replace @Data
with @Data
(see Logs section). It seems it happens only for Lombok @Data
, or @Setter
:
@Setter
public class ErrorProneTestObject {
@JsonProperty("event")
private String event;
public String getEvent() {
return event;
}
}
.../ErrorProneTestObject.java:6: Note: [CanonicalAnnotationSyntax] Omit redundant syntax from annotation declarations
@Data
^
(see https://error-prone.picnic.tech/bugpatterns/CanonicalAnnotationSyntax)
Did you mean '@Data'?
I tried several different combinations of the annotations and configurations and it seems that:
JsonProperty
's parameterI'm pretty sure that the error message does not help much so it is either a bug in the analysis or in the message. Does it sound familiar to you guys?
2.18.0
0.8.0
disableWarningsInGeneratedCode = true
in the configurationI just started using error-prone
and it might be a good opportunity for me to see how custom rules work. If you consider reported behavior a bug I think I could dive deeper into it
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>
[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<JavaAnnotation<?>> havingAttribute(checkArgument(@NonNull != @NonNull, @NonNull); String attributeName) {'?
The rule doesn't appear in the compiler output.
17.0.5
2.19.1
0.11.1
Specifying default behavior in a Reactor publisher can be written like switchIfEmpty(Mono.just(42))
or defaultIfEmpty(42)
.
The behavior is identical, but the latter is more concise and may have slight performance benefits due to reduced overhead.
I would like to rewrite the following code:
Mono<Integer> mono = ...;
return mono.switchIfEmpty(Mono.just(42));
to:
Mono<Integer> mono = ...;
return mono.defaultIfEmpty(42);
Mono
and Flux
.{Mono|Flux}.just(...)
. Cases like Mono.fromSupplier
cannot be rewritten because they would lose the lazy-evaluation behavior.switchIfEmpty(Mono.empty())
, in which case the switchIfEmpty
operator can simply be dropped.The CanonicalAnnotationSyntax
rule provides a wrong improvement message when there is a trailing comma on a list with > 1 elements.
@Import({
ClassA.class,
ClassB.class,
})
public final class ExampleConfig {}
[WARNING] /Users/dirkvanbokkem/Repositories/picnic-dwh-query-cache/dwh-query-cache/dwh-query-cache-web/src/main/java/tech/picnic/dwhquerycache/web/ExampleConfig.java:[5,1] [CanonicalAnnotationSyntax] Omit redundant syntax from annotation declarations
(see https://error-prone.picnic.tech/bugpatterns/CanonicalAnnotationSyntax)
Did you mean '@Import({'?
IIUC the CanonicalAnnotationSyntax
rule checks multiple things, including the following 2:
It's about 2., so the correct message would have been:
Did you mean 'ClassB.class'?
MacOS Sonoma 14.4
21.0.2
2.25.0
0.15.0
In case of only a single element, the improvement message is correct because it fits on one line directly so it can show both the curly braces and the trailing comma removals.
@Import({
ClassA.class,
})
public final class ExampleConfig {}
[WARNING] /Users/dirkvanbokkem/Repositories/picnic-dwh-query-cache/dwh-query-cache/dwh-query-cache-web/src/main/java/tech/picnic/dwhquerycache/web/ExampleConfig.java:[5,1] [CanonicalAnnotationSyntax] Omit redundant syntax from annotation declarations
(see https://error-prone.picnic.tech/bugpatterns/CanonicalAnnotationSyntax)
Did you mean '@Import(ClassA.class)'?
Duplicate entries inside annotation listing are a common mistake that should be flagged.
Introduce a new BugChecker to flag duplicate entries as this is a common mistake. Similar to LexicographicalAnnotationListing
, the new BugChecker should identify and handle duplicate entries to improve code quality and reduce potential errors.
I would like to rewrite the following code:
@A("foo", "bar", "foo")
A duplicateAnnotations();
to:
@A("foo", "bar")
A duplicateAnnotations();
π To consider: A duplicate entry is a duplicate pointer reference and not just the same name.
π Open question: Are there situations where we do not want to flag this ?
See this comment.
Edit 1: Update requirements based on this.
Sometimes a nullable variable can be wrapped in an Optional
simply to perform a null-or-else check.
In such cases it's simpler to use requireNonNullElse
or requireNonNullElseGet
from java.util.Objects
.
I would like to rewrite the following code:
Optional.ofNullable(A).orElse(B);
Optional.ofNullable(A).orElseGet(() -> getB());
to:
requireNonNullElse(A, B);
requireNonNullElseGet(A, () -> getB());
Could also consider the case Optional.of(A).orElse(B)
which can be rewritten as just A
, and similar for orElseGet
.
EDIT: actually, Optional.of
would already throw an exception if A
was null
, while replacing with just A
could propagate the null
further, so it should be replaced with requireNonNull(A)
.
In checkstyle/checkstyle#14229 we are enabling the CollectorMutability
check in the Checkstyle repository. The Checkstyle team tries to minimize the dependencies they have on Guava. Therefore, the rewrite to Immutable{List,Map,Set}.toImmutable{List,Map,Set}()
is not desirable. An alternative would be to use the Collectors.toUnmodifiable{List,Map,Set}()
. This is a good alternative as this also clearly communicates that a list is used that should not be modified.
I would like to rewrite the following code:
.collect(Collectors.toSet());
to:
.collect(Collectors.toUnmodifiableSet());
Two things:
ThirdPartyLibrary
should be resolved as well. @SentrySpan
@Transactional
@Scheduled(cron = "@hourly")
@SchedulerLock(name = SYNC_EXERCISES_LOCK_NAME, lockAtLeastFor = "2m", lockAtMostFor = "10m")
@SentryTransaction(operation = "task")
Produces:
[ERROR] (see https://error-prone.picnic.tech/bugpatterns/ScheduledTransactionTrace)
[ERROR] Did you mean '@Trace(dispatcher = true)@SentrySpan'?
No error should be present. We use Sentry (not New Relic).
None.
I couldn't find any documentation on how to apply the Refaster suggestions. The suggestions coming from Errorprone are applied as expected, but the Refaster ones don't and it is not obvious how to apply them. (Does it require your fork of Errorprone?)
Rewrite String#copyValueOf(char[])
to new String(char[])
.
The Refaster rule should live in the StringRules
file.
Make sure to add a test entry in the StringRulesTestInput
and StringRulesTestOutput π.
I would like to rewrite the following code:
char[] foo = new char[0];
String s = String.copyValueOf(foo);
to:
char[] foo = new char[0];
String s = new String(foo);
There are several types of Stream#collect(..)
calls for which a simpler and more canonical alternative exists. We should nudge users to use those features.
Concretely, the idea is to implement Refaster rules for each of the cases mentioned by SonarCloud rule S4266.
So to take the first example mentioned by the rule, we would like to rewrite the following code:
stream.collect(counting())
to:
stream.count()
comparator
or mapping
. Make sure that those Refaster rules use the maximally generic type, such as Comparator<? super T>
in case of comparator
. See the method signatures of the replaced methods for details.@SuppressWarnings("java:S4266")
to the new @BeforeTemplate
methods. See the changes in #575 for similar suppressions.While experimenting Error Prone Support on jbanking I got a java.lang.NullPointerException
in tech.picnic.errorprone.bugpatterns.ScheduledTransactionTrace
.
Considering the ScheduledTransactionTrace
bug pattern is located in error-prone-contrib I opened an issue here instead of https://github.com/google/error-prone.
The code that triggered the issue is located at https://github.com/marcwrobel/jbanking/blob/error-prone/src/main/java/fr/marcwrobel/jbanking/calendar/DayOfWeekInMonthHoliday.java. The bug can be reproduced using https://github.com/marcwrobel/jbanking/tree/error-prone.
git clone [email protected]:marcwrobel/jbanking.git
cd jbanking
git checkout error-prone
(error-prone u=)$ mvn clean install
Apache Maven 3.9.2 (c9616018c7a021c1c39be70fb2843d6f5f9b8a1c)
Maven home: /home/mwrobel/.asdf/installs/maven/3.9.2
Java version: 17.0.7, vendor: Eclipse Adoptium, runtime: /home/mwrobel/.asdf/installs/java/temurin-17.0.7+7
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "6.1.0-0.deb11.6-amd64", arch: "amd64", family: "unix"
911 [INFO] Error stacktraces are turned on.
1001 [INFO] Scanning for projects...
1215 [INFO]
1215 [INFO] -----------------------< fr.marcwrobel:jbanking >-----------------------
1215 [INFO] Building jbanking 4.2.0-SNAPSHOT
1215 [INFO] from pom.xml
1215 [INFO] --------------------------------[ jar ]---------------------------------
1396 [INFO]
1397 [INFO] --- clean:3.2.0:clean (default-clean) @ jbanking ---
1464 [INFO] Deleting /home/mwrobel/projects/jbanking/target
1465 [INFO]
1466 [INFO] --- flatten:1.3.0:clean (flatten.clean) @ jbanking ---
1744 [INFO] Deleting /home/mwrobel/projects/jbanking/.flattened-pom.xml
1745 [INFO]
1745 [INFO] --- enforcer:3.1.0:enforce (enforce-maven) @ jbanking ---
1855 [INFO]
1855 [INFO] --- formatter:2.16.0:format (default) @ jbanking ---
2010 [INFO] Using 'UTF-8' encoding to format source files.
2039 [INFO] Number of files to be formatted: 98
3400 [INFO] Successfully formatted: 0 file(s)
3400 [INFO] Fail to format: 0 file(s)
3400 [INFO] Skipped: 98 file(s)
3400 [INFO] Read only skipped: 0 file(s)
3400 [INFO] Approximate time taken: 1s
3400 [INFO]
3400 [INFO] --- resources:3.3.0:resources (default-resources) @ jbanking ---
3466 [INFO] Copying 0 resource
3467 [INFO] Copying 1 resource
3472 [INFO]
3473 [INFO] --- flatten:1.3.0:flatten (flatten) @ jbanking ---
3485 [INFO] Generating flattened POM of project fr.marcwrobel:jbanking:jar:4.2.0-SNAPSHOT...
3548 [INFO]
3548 [INFO] --- compiler:3.10.1:compile (default-compile) @ jbanking ---
3814 [INFO] Changes detected - recompiling the module!
3816 [INFO] Compiling 49 source files to /home/mwrobel/projects/jbanking/target/classes
6490 [INFO] -------------------------------------------------------------
6490 [WARNING] COMPILATION WARNING :
6490 [INFO] -------------------------------------------------------------
6490 [WARNING] bootstrap class path not set in conjunction with -source 8
6490 [INFO] 1 warning
6490 [INFO] -------------------------------------------------------------
6491 [INFO] -------------------------------------------------------------
6491 [ERROR] COMPILATION ERROR :
6491 [INFO] -------------------------------------------------------------
6491 [ERROR] /home/mwrobel/projects/jbanking/src/main/java/fr/marcwrobel/jbanking/calendar/DayOfWeekInMonthHoliday.java:[39,10] An unhandled exception was thrown by the Error Prone static analysis plugin.
Please report this at https://github.com/google/error-prone/issues/new and include the following:
error-prone version: 2.19.1
BugPattern: ScheduledTransactionTrace
Stack Trace:
java.lang.NullPointerException: Cannot invoke "java.util.Map.get(Object)" because "msym.visiblePackages" is null
at jdk.compiler/com.sun.tools.javac.code.Symtab.lookupPackage(Symtab.java:695)
at jdk.compiler/com.sun.tools.javac.code.Symtab.lookupPackage(Symtab.java:676)
at jdk.compiler/com.sun.tools.javac.code.ClassFinder.loadClass(ClassFinder.java:425)
at tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary.canLoadClass(ThirdPartyLibrary.java:91)
at tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary.isKnownClass(ThirdPartyLibrary.java:84)
at tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary.canIntroduceUsage(ThirdPartyLibrary.java:74)
at tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary.lambda$new$60d6954b$1(ThirdPartyLibrary.java:59)
at com.google.errorprone.VisitorState$Cache.get(VisitorState.java:702)
at tech.picnic.errorprone.bugpatterns.util.ThirdPartyLibrary.isIntroductionAllowed(ThirdPartyLibrary.java:70)
at tech.picnic.errorprone.bugpatterns.ScheduledTransactionTrace.matchMethod(ScheduledTransactionTrace.java:55)
at com.google.errorprone.scanner.ErrorProneScanner.processMatchers(ErrorProneScanner.java:449)
at com.google.errorprone.scanner.ErrorProneScanner.visitMethod(ErrorProneScanner.java:739)
at com.google.errorprone.scanner.ErrorProneScanner.visitMethod(ErrorProneScanner.java:150)
at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCMethodDecl.accept(JCTree.java:953)
at jdk.compiler/com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:86)
at com.google.errorprone.scanner.Scanner.scan(Scanner.java:74)
at com.google.errorprone.scanner.Scanner.scan(Scanner.java:48)
at jdk.compiler/com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:96)
at jdk.compiler/com.sun.source.util.TreeScanner.scan(TreeScanner.java:111)
at jdk.compiler/com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:119)
at jdk.compiler/com.sun.source.util.TreeScanner.visitClass(TreeScanner.java:203)
at com.google.errorprone.scanner.ErrorProneScanner.visitClass(ErrorProneScanner.java:548)
at com.google.errorprone.scanner.ErrorProneScanner.visitClass(ErrorProneScanner.java:150)
at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCClassDecl.accept(JCTree.java:860)
at jdk.compiler/com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:86)
at com.google.errorprone.scanner.Scanner.scan(Scanner.java:74)
at com.google.errorprone.scanner.Scanner.scan(Scanner.java:48)
at jdk.compiler/com.sun.source.util.TreeScanner.scan(TreeScanner.java:111)
at jdk.compiler/com.sun.source.util.TreeScanner.scanAndReduce(TreeScanner.java:119)
at jdk.compiler/com.sun.source.util.TreeScanner.visitCompilationUnit(TreeScanner.java:152)
at com.google.errorprone.scanner.ErrorProneScanner.visitCompilationUnit(ErrorProneScanner.java:560)
at com.google.errorprone.scanner.ErrorProneScanner.visitCompilationUnit(ErrorProneScanner.java:150)
at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCCompilationUnit.accept(JCTree.java:614)
at jdk.compiler/com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:60)
at com.google.errorprone.scanner.Scanner.scan(Scanner.java:58)
at com.google.errorprone.scanner.ErrorProneScannerTransformer.apply(ErrorProneScannerTransformer.java:43)
at com.google.errorprone.ErrorProneAnalyzer.finished(ErrorProneAnalyzer.java:156)
at jdk.compiler/com.sun.tools.javac.api.MultiTaskListener.finished(MultiTaskListener.java:132)
at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1394)
at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1341)
at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:933)
at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.lambda$doCall$0(JavacTaskImpl.java:104)
at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.invocationHelper(JavacTaskImpl.java:152)
at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.doCall(JavacTaskImpl.java:100)
at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.call(JavacTaskImpl.java:94)
at org.codehaus.plexus.compiler.javac.JavaxToolsCompiler.compileInProcess(JavaxToolsCompiler.java:136)
at org.codehaus.plexus.compiler.javac.JavacCompiler.performCompile(JavacCompiler.java:182)
at org.apache.maven.plugin.compiler.AbstractCompilerMojo.execute(AbstractCompilerMojo.java:1209)
at org.apache.maven.plugin.compiler.CompilerMojo.execute(CompilerMojo.java:198)
at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:126)
at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2(MojoExecutor.java:342)
at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute(MojoExecutor.java:330)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:213)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:175)
at org.apache.maven.lifecycle.internal.MojoExecutor.access$000(MojoExecutor.java:76)
at org.apache.maven.lifecycle.internal.MojoExecutor$1.run(MojoExecutor.java:163)
at org.apache.maven.plugin.DefaultMojosExecutionStrategy.execute(DefaultMojosExecutionStrategy.java:39)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:160)
at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:105)
at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:73)
at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build(SingleThreadedBuilder.java:53)
at org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:118)
at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:261)
at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:173)
at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:101)
at org.apache.maven.cli.MavenCli.execute(MavenCli.java:910)
at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:283)
at org.apache.maven.cli.MavenCli.main(MavenCli.java:206)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:283)
at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:226)
at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:407)
at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:348)
6492 [INFO] 1 error
6492 [INFO] -------------------------------------------------------------
6492 [INFO] ------------------------------------------------------------------------
6493 [INFO] BUILD FAILURE
6493 [INFO] ------------------------------------------------------------------------
6494 [INFO] Total time: 5.559 s
6494 [INFO] Finished at: 2023-05-14T18:32:32+02:00
6494 [INFO] ------------------------------------------------------------------------
A successful build, without NullPointerException
.
None
The IsInstanceLambdaUsage
check works for most cases. However, when we invoke a method on the object passed to the filter, the suggestion is incorrect, which leads to non-compilable code.
This code:
Stream.of(1).filter(i -> i.getClass() instanceof Class);
// Given `locations` is a list of `Location` objects.
Stream.of(locations).filter(location -> location.getName() instanceof String);
Is rewritten to:
Stream.of(1).filter(Class.class::isInstance);
// Given `locations` is a list of `Location` objects.
Stream.of(locations).filter(String.class::isInstance);
This is not behavior preserving, as we are now checking whether i
is an instance of Class
. This usually leads to non-compilable code. To clarify, in the second example we are now checking that location
is an instance of String
instead of the location.getName()
.
I think the best option for now would be to not flag these cases. We probably can't make a clear improvement here.
Linux Ubuntu 20.04
openjdk 17.0.3 2022-04-19 LTS
2.16
.0.5.0
.We want to sort the order of the members in a class.
Internally we are quite strict with our ordering for the sake of consistency.
The following order should be enforced in a class:
Create an Error Prone check that changes the order of the members of a class without altering the empty lines between the members of the class.
I would like to rewrite the following code:
class A {
char a = 'a';
private static String FOO = "foo";
static int ONE = 1;
void m2() {}
public A () {}
private static String BAR = "bar";
char b = 'b';
void m1() {}
static int TWO = 2;
}
to:
class A {
private final static String FOO = "foo";
static int ONE = 1;
private final static String BAR = "bar";
static int TWO = 2;
char a = 'a';
char b = 'b';
public A () {}
void m1() {}
void m2() {}
}
SuggestedFix#replace(Tree tree, String replaceWith)
is used. Canonicalization of whitespace will be handles by a separate check.As @EnricSala mentioned in #364, we might want to investigate rewriting Optional.of(A).orElse(B)
to requireNonNull(A)
. This will probably require a BugChecker
as we probably want to provide two fixes, have to do a more extensive analysis, and are making some non behavior preserving changes.
We should also consider the Optional#orElseGet
case.
I would like to rewrite the following code:
Optional.of(1).orElse(2); // Holds for non-nullable things.
Optional.of(null).orElse(2); // Holds for nullable things.
to:
requireNonNull(1); // Holds for non-nullable things.
Optional.ofNullable(null).orElse(2); // Holds for nullable things.
Given a stream of elements that must be filtered and sorted, it is generally more efficient to do the former first, so that the subsequent sorting step involves fewer elements. This applies to Stream
, IntStream
, LongStream
, DoubleStream
and Flux
types.
Rewrite:
stream.sorted().filter(pred)
to:
stream.filter(pred).sorted()
Do the same for the other types mentioned above, as well as Comparator
-accepting sort method overloads.
One nice advantage of the proposed rewrite rules is that these simplifications might subsequently kick in.
Reactor's PublisherProbe
defines a neat API to test reactive code. However, it is only helpful when it is actually used.
Considering the following code snippet, the PublisherProbe
is initialized and asserted, but never actually used. Surely we can assert that it was never subscribed to, because we never included it in code that should be subscribed to. π
@Test
void probeBugChecker() {
PublisherProbe<Void> probe = PublisherProbe.empty();
// some test code that doesn't involve `probe`
probe.assertWasNotSubscribed();
}
This looks like an unintentional human mistake that wrongfully could increase the author's confidence in their production code.
I'd like to have a BugChecker that identifies such a construct as a common mistake and expects changes.
In essence, this BugChecker should flag in the following scenario:
PublisherProbe
is declared and initializedPublisherProbe#assertWasNotSubscribed
).The BugChecker should not flag if
PublisherProbe
is used
PublisherProbe#{mono,flux}
Most likely not within the next couple of weeks.
Methods returning reactive types but in reality are implemented synchronously give callers a wrong impression of what's going on in a method. See this section of the "Avoiding Reactor Meltdown" talk describing such methods.
The following is such example. imposterMethod()
has a reactive method declaration but isn't doing anything that warrants this. This will result in unnecessary upstream subscriptions.
class Example {
record SetHolder(Set<Integer> values) {}
static Mono<SetHolder> imposterMethod(Set<Integer> values) {
return Mono.just(values)
.filter(Predicate.not(Set::isEmpty))
.doOnNext(set -> LOG.debug("Found example set {}", set))
.map(SetHolder::new);
}
}
I'd like to have a BugChecker that identifies such a construct as a common mistake and expects changes.
If the method has a reactive chain that starts with {Mono,Flux}#just
and calls no method that results in an inner subscription (e.g. Mono#flatMap()
), then we should have a Reactor imposter method and such should be flagged.
I'm opening this issue to start a discussion (a) whether this is desired, and (b) how to achieve such a BugChecker.
I can see it guide new-comers to better usage of reactive operators.
I was attempting to run the latest version of the rules using the version of errorprone built in to bazel 5.2.1. This failed because it appears the latest version only works with versions of errorprone > 2.20.0 (or so). I was not able to find any documentation of that, but there are some closed issues where people had similar problems. It may make sense to have a compatibility matrix or something prominent in the documentation.
We currently do not have a way to disable these rules as we do not use guava on our codebase. Is there a way to disable these globaly without using annotations SuppressWarnings(...)
. An example that triggers the warning.
var array = List.of(....)
It is possible to use ./apply-error-prone-suggestions.sh to apply suggestions provided by EPS but it requires to checkout and build EPS locally, presenting an additional hindrance/inconvenience.
I'd like to apply all or most suggestions provided by EP(S) without having to check out build EPS as it is mentioned in the readme.
In particular newly adopting projects would benefit from a quick & easy way to view EPS' suggestions in the resulting git diff.
In essence, I am looking for the convenience equivalent of fmt-maven-plugin's mvn fmt:format
feature, configurable by bug check/Refaster rule.
Error Prone itself has experimental support for creating patches. Potentially, all that is required is to improve EPS' documentation.
Using JUnit 5's Arguments
API performs validation of its parameters at runtime which can lead to three scenarios:
I would like to warn about the following code:
private static Stream<Arguments> sumTestCases() {
/* { value1, value2, result } */
return Stream.of(
arguments(1, 1, 2), // Correct test case.
arguments(
1,
1,
// (1) Wrong type; results in a `ParameterResolutionException`, i.e. runtime exception.
"Irrelevant parameter"),
arguments(
1,
1,
2,
// (3) Superfluous last parameter does not result in an error.
Optional.empty()));
}
@ParameterizedTest
// (1) Wrong type; results in a `ParameterResolutionException`, i.e. runtime exception.
@EnumSource(java.time.DayOfWeek.class)
// (2) Insufficient number of parameters.
@ValueSource(ints = {1, 2, 3})
@MethodSource("sumTestCases")
void sum(int value1, int value2, int result) {
assertThat(value1 + value2).isEqualTo(result);
}
(1) and (2) are caught when executing the tests themselves, but (3) is not. Such superfluous parameters result in misleading test cases. Out of the three scenarios, (3) is IMO the one that should be tackled first. However, when adding parameter validation, why only do (3) when the other two are probably not much more work.
I most likely won't get to this in the near future.
Summary says it; this is a good candidate for a Refaster rule.
Use Refaster to rewrite code of the form:
collection.stream().forEach(consumer);
to
collection.forEach(consumer);
In theory this rewrite rule could change the code's semantics, but this is unlikely in practice.
As part of the preparation for the workshop i was cloning this repo and setting up versions and stuff.
Requirements (Important: make sure you have this ready on your laptop before the start of the workshop):
workshop
branch.mvn clean install
in the root of the repository and get a successful build.workshop
branch. 244ac55mvn clean install
in the root of the repositoryCaused by: org.eclipse.aether.resolution.ArtifactResolutionException:
The following artifacts could not be resolved:
tech.picnic.error-prone-support:error-prone-support:pom:0.14.1-SNAPSHOT (absent):
mvn -v
Apache Maven 3.9.6 (bc0240f3c744dd6b6ec2920b3cd08dcc295161ae)
Maven home: ~/.sdkman/candidates/maven/3.9.6
Java version: 17.0.9, vendor: Eclipse Adoptium, runtime: ~/.sdkman/candidates/java/17.0.9-tem
Default locale: nl_NL, platform encoding: UTF-8
OS name: "mac os x", version: "14.4.1", arch: "aarch64", family: "mac"
This will probably be something other people also run in to for the meetup https://www.meetup.com/codelabjug/events/299169758/
Declaring a Matcher
using Matchers.{allOf,anyOf}(Matcher...)
containing just a single Matcher
, is redundant.
We can rewrite
allOf(annotations(AT_LEAST_ONE, isType("org.junit.jupiter.params.provider.MethodSource")));
anyOf(annotations(AT_LEAST_ONE, isType("org.junit.jupiter.params.provider.MethodSource")));
to:
annotations(AT_LEAST_ONE, isType("org.junit.jupiter.params.provider.MethodSource"));
annotations(AT_LEAST_ONE, isType("org.junit.jupiter.params.provider.MethodSource"));
This should support the method overloads Matchers.{allOf,anyOf}(Iterable<? extends Matcher>)
.
This is a nice opportunity to get your hands dirty with BugChecker
s π
In the google format guide, wildcard imports aren't allowed.
It would be nice if a refaster rule could convert * imports to single imports.
However, it's not immediately clear to me if a refaster template could even handle such a case.
What do you think?
Internally we heavily use SLF4J for logging in our classes.
We would like to have a check to canonicalize the usage of such loggers.
For that reason we would like a check that looks at the following things:
private static final
?LOG
?LoggerFactory#getLogger
?I would like to rewrite the following code:
final class SimpleExample {
public Logger LOGGER = LoggerFactory.getLogger(WrongClass.class);
...
}
to:
final class SimpleExample {
private static final Logger LOG = LoggerFactory.getLogger(SimpleExample.class);
...
}
For interfaces we have to make an exception when it comes to the private static final
, as in interfaces this is not an option.
As raised by @mlrprananta himself, we could parameterize the variable name of the logger. Perhaps we can add an option that people can override the allowed variable name of the logger via the ErrorProneFlags
.
For this check we don't need to look at the line on which the logger is defined, we will do that in a different check.
This project has some related checks KengoTODA/errorprone-slf4j.
However, we want a check that is specific to our internal standards. Additionlly, in EPS we have our own best practices that we want to use for the code of this check.
Later, we might want to add more features to this check. This would be a good first version π.
When upgrading to the latest release (2.29.2 to 2.30.0) it fails due
Caused by: java.lang.IllegalStateException: Can't load
CodeTransformer
from tech/picnic/errorprone/refasterrules/AssertJBigDecimalRules$AbstractBigDecimalAssertIsEqualByComparingTo.refaster
Caused by: java.io.InvalidClassException: com.google.errorprone.refaster.RefasterRule; local class incompatible: stream classdesc serialVersionUID = 4962906561874863273, local class serialVersionUID = 4787521360352653288
See the gradle build scan or the error log below.
> Task :caffeine:compileJavaPoetJava FAILED
An exception has occurred in the compiler (17.0.11). Please file a bug against the Java compiler via the Java bug reporting page (https://bugreport.java.com) after checking the Bug Database (https://bugs.java.com) for duplicates. Include your program, the following diagnostic, and the parameters passed to the Java compiler in your report. Thank you.
java.lang.AssertionError: com.google.errorprone.scanner.ErrorProneInjector$ProvisionException: Failed to initialize tech.picnic.errorprone.refaster.runner.Refaster
at com.google.errorprone.ErrorProneAnalyzer.lambda$scansPlugins$2(ErrorProneAnalyzer.java:159)
at com.google.common.base.Suppliers$NonSerializableMemoizingSupplier.get(Suppliers.java:186)
at com.google.errorprone.ErrorProneAnalyzer.finished(ErrorProneAnalyzer.java:223)
at jdk.compiler/com.sun.tools.javac.api.MultiTaskListener.finished(MultiTaskListener.java:132)
at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1397)
at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1344)
at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:933)
at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.lambda$doCall$0(JavacTaskImpl.java:104)
at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.invocationHelper(JavacTaskImpl.java:152)
at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.doCall(JavacTaskImpl.java:100)
at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.call(JavacTaskImpl.java:94)
at org.gradle.internal.compiler.java.IncrementalCompileTask.call(IncrementalCompileTask.java:92)
at org.gradle.api.internal.tasks.compile.AnnotationProcessingCompileTask.call(AnnotationProcessingCompileTask.java:94)
at org.gradle.api.internal.tasks.compile.ResourceCleaningCompilationTask.call(ResourceCleaningCompilationTask.java:57)
at org.gradle.api.internal.tasks.compile.JdkJavaCompiler.execute(JdkJavaCompiler.java:76)
at org.gradle.api.internal.tasks.compile.JdkJavaCompiler.execute(JdkJavaCompiler.java:46)
at org.gradle.api.internal.tasks.compile.daemon.AbstractIsolatedCompilerWorkerExecutor$CompilerWorkAction.execute(AbstractIsolatedCompilerWorkerExecutor.java:78)
at org.gradle.workers.internal.DefaultWorkerServer.execute(DefaultWorkerServer.java:63)
at org.gradle.workers.internal.AbstractClassLoaderWorker$1.create(AbstractClassLoaderWorker.java:54)
at org.gradle.workers.internal.AbstractClassLoaderWorker$1.create(AbstractClassLoaderWorker.java:48)
at org.gradle.internal.classloader.ClassLoaderUtils.executeInClassloader(ClassLoaderUtils.java:100)
at org.gradle.workers.internal.AbstractClassLoaderWorker.executeInClassLoader(AbstractClassLoaderWorker.java:48)
at org.gradle.workers.internal.FlatClassLoaderWorker.run(FlatClassLoaderWorker.java:32)
at org.gradle.workers.internal.FlatClassLoaderWorker.run(FlatClassLoaderWorker.java:22)
at org.gradle.workers.internal.WorkerDaemonServer.run(WorkerDaemonServer.java:103)
at org.gradle.workers.internal.WorkerDaemonServer.run(WorkerDaemonServer.java:72)
at org.gradle.process.internal.worker.request.WorkerAction$1.call(WorkerAction.java:152)
at org.gradle.process.internal.worker.child.WorkerLogEventListener.withWorkerLoggingProtocol(WorkerLogEventListener.java:41)
at org.gradle.process.internal.worker.request.WorkerAction.lambda$run$1(WorkerAction.java:149)
at org.gradle.internal.operations.CurrentBuildOperationRef.with(CurrentBuildOperationRef.java:85)
at org.gradle.process.internal.worker.request.WorkerAction.run(WorkerAction.java:141)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:182)
at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:164)
at org.gradle.internal.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:414)
at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64)
at org.gradle.internal.concurrent.AbstractManagedExecutor$1.run(AbstractManagedExecutor.java:48)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
at java.base/java.lang.Thread.run(Thread.java:840)
Caused by: com.google.errorprone.scanner.ErrorProneInjector$ProvisionException: Failed to initialize tech.picnic.errorprone.refaster.runner.Refaster
at com.google.errorprone.scanner.ErrorProneInjector.getInstance(ErrorProneInjector.java:92)
at com.google.errorprone.scanner.ErrorProneInjector.getInstance(ErrorProneInjector.java:65)
at com.google.errorprone.scanner.ScannerSupplierImpl.instantiateChecker(ScannerSupplierImpl.java:66)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:992)
at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
at com.google.errorprone.scanner.ScannerSupplierImpl.get(ScannerSupplierImpl.java:74)
at com.google.errorprone.scanner.ScannerSupplierImpl.get(ScannerSupplierImpl.java:37)
at com.google.errorprone.ErrorProneAnalyzer.lambda$scansPlugins$2(ErrorProneAnalyzer.java:154)
... 44 more
Caused by: java.lang.reflect.InvocationTargetException
at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
at com.google.errorprone.scanner.ErrorProneInjector.getInstance(ErrorProneInjector.java:90)
... 56 more
Caused by: java.lang.IllegalStateException: Can't load `CodeTransformer` from tech/picnic/errorprone/refasterrules/AssertJBigDecimalRules$AbstractBigDecimalAssertIsEqualByComparingTo.refaster
at tech.picnic.errorprone.refaster.runner.CodeTransformers.loadCodeTransformer(CodeTransformers.java:106)
at tech.picnic.errorprone.refaster.runner.CodeTransformers.lambda$loadAllCodeTransformers$1(CodeTransformers.java:53)
at java.base/java.util.Optional.ifPresent(Optional.java:178)
at tech.picnic.errorprone.refaster.runner.CodeTransformers.loadAllCodeTransformers(CodeTransformers.java:51)
at com.google.common.base.Suppliers$NonSerializableMemoizingSupplier.get(Suppliers.java:186)
at tech.picnic.errorprone.refaster.runner.CodeTransformers.getAllCodeTransformers(CodeTransformers.java:36)
at tech.picnic.errorprone.refaster.runner.Refaster.createCompositeCodeTransformer(Refaster.java:198)
at tech.picnic.errorprone.refaster.runner.Refaster.<init>(Refaster.java:83)
... 62 more
Caused by: java.io.InvalidClassException: com.google.errorprone.refaster.RefasterRule; local class incompatible: stream classdesc serialVersionUID = 4962906561874863273, local class serialVersionUID = 4787521360352653288
at java.base/java.io.ObjectStreamClass.initNonProxy(ObjectStreamClass.java:597)
at java.base/java.io.ObjectInputStream.readNonProxyDesc(ObjectInputStream.java:2051)
at java.base/java.io.ObjectInputStream.readClassDesc(ObjectInputStream.java:1898)
at java.base/java.io.ObjectInputStream.readNonProxyDesc(ObjectInputStream.java:2051)
at java.base/java.io.ObjectInputStream.readClassDesc(ObjectInputStream.java:1898)
at java.base/java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2224)
at java.base/java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1733)
at java.base/java.io.ObjectInputStream.readArray(ObjectInputStream.java:2157)
at java.base/java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1721)
at java.base/java.io.ObjectInputStream$FieldValues.<init>(ObjectInputStream.java:2606)
at java.base/java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:2457)
at java.base/java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2257)
at java.base/java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1733)
at java.base/java.io.ObjectInputStream$FieldValues.<init>(ObjectInputStream.java:2606)
at java.base/java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:2457)
at java.base/java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2257)
at java.base/java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1733)
at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:509)
at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:467)
at tech.picnic.errorprone.refaster.runner.CodeTransformers.loadCodeTransformer(CodeTransformers.java:89)
... 69 more
Running the JUnitToAssertJRules
across some code I noticed that AssertThatIsTrue
and AssertThatIsFalse
transformed some JUnit assertTrue()
and assertFalse()
calls:
It seems like it would be a nice addition to have specific rules calling AbstractIteratorAssert#hasNext()
and AbstractIteratorAssert#isExhausted()
instead.
The example was found in hibernate/hibernate-tools: https://github.com/hibernate/hibernate-tools/blob/10431fc27866d295be9d2062ffdc9b1cd9099c16/jbt/src/test/java/org/hibernate/tool/orm/jbt/util/NativeConfigurationTest.java#L242-L247
According to the Google Java Format Style guide constants names should use UPPER_SNAKE_CASE
.
There is a ConstantField
check in Error Prone that does the following:
When naming a field with
CONSTANT_CASE
, make sure the field isstatic
,final
, and of immutable type. If the field doesnβt meet those criteria, uselowerCamelCase
instead.
We want to go one step further and ensure that all constant names are UPPER_SNAKE_CASE
.
I would like to rewrite the following code:
private static final int number = 1;
static final int otherNumber = 2;
to:
private static final int NUMBER = 1;
static final int OTHER_NUMBER = 2;
Carefully read the GJF Style Guide on constant names and maybe use examples from there.
This check might require some additional analysis for static
fields that are mutable. This should be a rare case though. Let's see how we handle this when we get there π.
When calling String#to{Lower,Upper}Case
without arguments, the system's Locale
is used by default.
To prevent this we want our code to specify Locale.ROOT
.
Therefore, we want to rewrite occurrences that invoke the aforementioned methods without Locale
and introduce Locale.ROOT
.
I would like to rewrite the following code:
"A".toLowerCase();
"B".toUpperCase();
to:
"A".toLowerCase(Locale.ROOT);
"B".toUpperCase(Locale.ROOT);
Just a consideration; instead of Locale.ROOT
one can also use other Locale
s. I assume we want to leave those as is, right?
This is a nice opportunity for contributing an Error Prone check π.
Thanks for the great package of checks! I tried to apply ErrorProneTestHelperSourceFormat and its autofixes to our tests in NullAway, and saw that blank lines get inserted as ""
. E.g.:
msridhar/NullAway@7c8ac63#diff-409faf90d91de636986e6f96aabde20a120b5442cbc3a32af4db4ca837915fceR21
Would it be possible for the check to not require these blank lines, and for the auto-fix not to insert them? Alternately, could you describe your workflow for writing tests such that the developer does not need to manually write these lines? Do you have a good way to write these tests without manually writing and indenting source lines enclosed in quotes? Thanks so much!
Excessive blocking is a known pitfall in reactive programming. Some of Reactor's blocking operators are quite explicit about their behavior, e.g. Mono#block
and Flux#blockFirst
, leaving no doubt that the operation is in fact blocking.
This does not apply to two of the convenience methods exposed by Reactor's Flux
, Flux#toStream and Flux#toIterable. These operators are documented to block, but this is not apparent from the method signature. It is easy to miss that the operation is blocking despite any potential lazy consumption of the resulting Stream
or Iterable
.
We should rewrite the problematic operators in terms of explicitly blocking operators, highlighting actual behavior.
Focusing first on the stream, I would like to rewrite the following code:
flux.toStream()
to:
flux.collect(toList()).block().stream()
To be determined what a general rewrite for Flux#toIterable
would look like, or if this is actually desirable.
Willing, sure! Less sure on able. Would be happy to see anyone else submit a pull request to this effect. :D
This is specific to the BugCheckerRefactoringTestHelper
which is used in replacement tests for BugPatterns.
There are two TestMode
s, namely AST_MATCH
and TEXT_MATCH
. During multiple code reviews we noticed that using AST_MATCH
can hide bugs. Therefore we always prefer TEXT_MATCH
.
Besides that, we do not like the statically imported TEXT_MATCH
, as it results in having a little less context. Therefore we want to rewrite all these occurrences as well, using Refaster.
We would like to rewrite the following code using Refaster:
refactoringTestHelper.addInputLines("A.java", "").addOutputLines("A.java", "").doTest();
refactoringTestHelper.addInputLines("B.java", "").addOutputLines("A.java", "").doTest(TestMode.AST_MATCH);
refactoringTestHelper.addInputLines("C.java", "").addOutputLines("A.java", "").doTest(TEXT_MATCH);
refactoringTestHelper.addInputLines("C.java", "").addOutputLines("A.java", "").doTest(BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH);
to:
refactoringTestHelper.addInputLines("A.java", "").addOutputLines("A.java", "").doTest(TestMode.TEXT_MATCH);
refactoringTestHelper.addInputLines("B.java", "").addOutputLines("B.java", "").doTest(TestMode.TEXT_MATCH);
refactoringTestHelper.addInputLines("C.java", "").addOutputLines("C.java", "").doTest(TestMode.TEXT_MATCH);
refactoringTestHelper.addInputLines("C.java", "").addOutputLines("C.java", "").doTest(TestMode.TEXT_MATCH);
The rewrite from TestMode.AST_MATCH
to TestMode.TEXT_MATCH
is not a behavior preserving one. It's important to make a note about that by using @Description
on the Refaster rule.
In commit google/error-prone@570f0bf the DescribeMatch
BugChecker
got removed from Error Prone with the explanation that it got ported to Refaster. As we cannot see these kind of changes, we have to write these Refaster rules ourselves π.
I would like to rewrite the following code:
return buildDescription(tree).addFix(fix).build();
to:
return describeMatch(tree, fix);
In the test code of the deleted BugChecker
there is an edge case where it shouldn't propose a fix in the abstract class BugChecker {
itself. However, to be honest, I don't think we need to take this case into account.
We might need to introduce a new Refaster rule collection for this Refaster rule or list it under AssortedRules
.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
π Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. πππ
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google β€οΈ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.