-
Notifications
You must be signed in to change notification settings - Fork 2
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
Have InvalidParam
support records
#35
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -807,6 +807,29 @@ private static boolean isFinal(Symbol symbol) { | |||||||||||||
return (symbol.flags() & Flags.FINAL) == Flags.FINAL; | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
/** | ||||||||||||||
* Flag for record types, canonical record constructors and type members that are part of a | ||||||||||||||
* record's state vector. Can be replaced by {@code com.sun.tools.javac.code.Flags.RECORD} once | ||||||||||||||
* the minimum JDK version is 14. | ||||||||||||||
*/ | ||||||||||||||
private static final long RECORD_FLAG = 1L << 61; | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Although I really like the suggestion, I used this one-liner because it's used in the Error Prone project twice like that and wanted to be consistent. So I'd lean towards leaving it as-is. (When we use this in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh I misread, this is in the |
||||||||||||||
|
||||||||||||||
/** | ||||||||||||||
* Returns true if the given {@link Tree} is a record, a record's canonical constructor or a | ||||||||||||||
* member that is part of a record's state vector. | ||||||||||||||
*/ | ||||||||||||||
public static boolean isRecord(Tree tree) { | ||||||||||||||
return isRecord(getSymbol(tree)); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
/** | ||||||||||||||
* Returns true if the given {@link Symbol} is a record, a record's canonical constructor or a | ||||||||||||||
* member that is part of a record's state vector. | ||||||||||||||
*/ | ||||||||||||||
public static boolean isRecord(Symbol symbol) { | ||||||||||||||
return symbol != null && (symbol.flags() & RECORD_FLAG) == RECORD_FLAG; | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
/** | ||||||||||||||
* Determines whether a symbol has an annotation of the given type. This includes annotations | ||||||||||||||
* inherited from superclasses due to {@code @Inherited}. | ||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ | |
package com.google.errorprone.bugpatterns.javadoc; | ||
|
||
import static com.google.common.collect.ImmutableSet.toImmutableSet; | ||
import static com.google.common.collect.MoreCollectors.onlyElement; | ||
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; | ||
import static com.google.errorprone.bugpatterns.javadoc.Utils.diagnosticPosition; | ||
import static com.google.errorprone.bugpatterns.javadoc.Utils.getBestMatch; | ||
|
@@ -26,13 +27,15 @@ | |
import static com.google.errorprone.names.LevenshteinEditDistance.getEditDistance; | ||
|
||
import com.google.common.collect.ImmutableSet; | ||
import com.google.common.collect.MoreCollectors; | ||
import com.google.common.collect.Sets; | ||
import com.google.errorprone.BugPattern; | ||
import com.google.errorprone.VisitorState; | ||
import com.google.errorprone.bugpatterns.BugChecker; | ||
import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher; | ||
import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher; | ||
import com.google.errorprone.matchers.Description; | ||
import com.google.errorprone.util.ASTHelpers; | ||
import com.sun.source.doctree.DocTree; | ||
import com.sun.source.doctree.DocTree.Kind; | ||
import com.sun.source.doctree.LiteralTree; | ||
|
@@ -74,11 +77,18 @@ public final class InvalidParam extends BugChecker implements ClassTreeMatcher, | |
public Description matchClass(ClassTree classTree, VisitorState state) { | ||
DocTreePath path = getDocTreePath(state); | ||
if (path != null) { | ||
ImmutableSet<String> parameters = ImmutableSet.of(); | ||
ImmutableSet<String> parameters = | ||
ASTHelpers.isRecord(classTree) | ||
? getCanonicalRecordConstructor(classTree).getParameters().stream() | ||
.map(p -> p.getName().toString()) | ||
.collect(toImmutableSet()) | ||
: ImmutableSet.of(); | ||
|
||
ImmutableSet<String> typeParameters = | ||
classTree.getTypeParameters().stream() | ||
.map(t -> t.getName().toString()) | ||
.collect(toImmutableSet()); | ||
|
||
new ParamsChecker(state, classTree, parameters, typeParameters).scan(path, null); | ||
} | ||
return Description.NO_MATCH; | ||
|
@@ -101,6 +111,14 @@ public Description matchMethod(MethodTree methodTree, VisitorState state) { | |
return Description.NO_MATCH; | ||
} | ||
|
||
private static MethodTree getCanonicalRecordConstructor(ClassTree classTree) { | ||
return classTree.getMembers().stream() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For records in some cases the generated constructor could not be found by using Perhaps that is a bigger issue we should solve. However, I think that is out of scope for this PR. |
||
.filter(MethodTree.class::isInstance) | ||
.map(MethodTree.class::cast) | ||
.filter(ASTHelpers::isRecord) | ||
.collect(onlyElement()); | ||
} | ||
|
||
/** Checks that documented parameters match the method's parameter list. */ | ||
private final class ParamsChecker extends DocTreePathScanner<Void, Void> { | ||
private final VisitorState state; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't use
Flags.RECORD
yet, because that would fail with JDK 11...