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

Don't fire TooManyParameters on records. #4660

Merged
merged 1 commit into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static com.google.errorprone.matchers.Description.NO_MATCH;
import static com.google.errorprone.util.ASTHelpers.getSymbol;
import static com.google.errorprone.util.ASTHelpers.hasAnnotation;
import static com.google.errorprone.util.ASTHelpers.isRecord;
import static com.google.errorprone.util.ASTHelpers.methodIsPublicAndNotAnOverride;

import com.google.common.collect.ImmutableSet;
Expand Down Expand Up @@ -84,12 +85,13 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
return NO_MATCH;
}

String name = getSymbol(tree).isConstructor() ? "constructor" : "method";
String message =
String.format(
"Consider using a builder pattern instead of a method with %s parameters. Data shows"
+ " that defining methods with > 5 parameters often leads to bugs. See also"
"Consider using a builder pattern instead of a %s with %s parameters. Data shows"
+ " that defining %s with > 5 parameters often leads to bugs. See also"
+ " Effective Java, Item 2.",
paramCount);
name, paramCount, name);
return buildDescription(tree).setMessage(message).build();
}

Expand All @@ -100,6 +102,9 @@ private static boolean shouldApplyApiChecks(MethodTree tree, VisitorState state)
.anyMatch(a -> hasAnnotation(symbol.owner, a, state))) {
return false;
}
if (isRecord(symbol)) {
return false;
}
return METHOD_ANNOTATIONS_TO_IGNORE.stream().noneMatch(a -> hasAnnotation(tree, a, state))
&& methodIsPublicAndNotAnOverride(symbol, state);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,20 @@ private ConstructorTest(int a, int b, int c, int d, int e, int f, int g) {}
.doTest();
}

@Test
public void recordConstructor() {
compilationHelper
.setArgs(ImmutableList.of("-XepOpt:" + TOO_MANY_PARAMETERS_FLAG_NAME + "=3"))
.addSourceLines(
"RecordExample.java",
"""
public record RecordExample(int p0, int p1, int p2, int p3, int p4, int p5) {
public RecordExample {}
}
""")
.doTest();
}

@Test
public void constructor_withAtInject() {
compilationHelper
Expand Down Expand Up @@ -114,6 +128,7 @@ public void ignoresAutoFactory() {
"AutoFactory.java",
"""
package com.google.auto.factory;
public @interface AutoFactory {}
""")
.addSourceLines(
Expand Down
Loading