Skip to content

Commit

Permalink
Suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
Stephan202 committed Oct 9, 2022
1 parent 0157692 commit 72f8881
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 28 deletions.
3 changes: 0 additions & 3 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1296,9 +1296,6 @@
<threads>4</threads>
<timeoutFactor>4</timeoutFactor>
<timestampedReports>false</timestampedReports>
<excludedClasses>
<excludedClass>*.AutoValue_*</excludedClass>
</excludedClasses>
</configuration>
<dependencies>
<dependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Maps;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -27,9 +26,9 @@ abstract class Node<T> {
// there be such an overload?
static <T> Node<T> create(
Set<T> values, Function<? super T, ? extends Set<? extends Set<String>>> pathExtractor) {
BuildNode<T> tree = BuildNode.create();
Builder<T> tree = Builder.create();
tree.register(values, pathExtractor);
return tree.immutable();
return tree.build();
}

abstract ImmutableMap<String, Node<T>> children();
Expand Down Expand Up @@ -75,12 +74,12 @@ private void collectReachableValues(ImmutableList<String> candidateEdges, Consum

@AutoValue
@SuppressWarnings("AutoValueImmutableFields" /* Type is used only during `Node` construction. */)
abstract static class BuildNode<T> {
private static <T> BuildNode<T> create() {
return new AutoValue_Node_BuildNode<>(new HashMap<>(), new ArrayList<>());
abstract static class Builder<T> {
private static <T> Builder<T> create() {
return new AutoValue_Node_Builder<>(new HashMap<>(), new ArrayList<>());
}

abstract Map<String, BuildNode<T>> children();
abstract Map<String, Builder<T>> children();

abstract List<T> values();

Expand All @@ -98,7 +97,7 @@ private void register(
* We sort paths by length ascending, so that in case of two paths where one is an initial
* prefix of the other, only the former is encoded (thus saving some space).
*/
Collections.sort(paths, comparingInt(Set::size));
paths.sort(comparingInt(Set::size));
paths.forEach(path -> registerPath(value, ImmutableList.sortedCopyOf(path)));
}
}
Expand All @@ -118,9 +117,9 @@ private void registerPath(T value, ImmutableList<String> path) {
}
}

private Node<T> immutable() {
private Node<T> build() {
return new AutoValue_Node<>(
ImmutableMap.copyOf(Maps.transformValues(children(), BuildNode::immutable)),
ImmutableMap.copyOf(Maps.transformValues(children(), Builder::build)),
ImmutableList.copyOf(values()));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,6 @@ public Refaster(ErrorProneFlags flags) {
public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState state) {
Set<CodeTransformer> candidateTransformers = ruleSelector.selectCandidateRules(tree);

// XXX: Remove these debug lines
// String removeThis = candidateRules.stream().map(Object::toString).collect(joining(","));
// System.out.printf("\n---Templates for %s: \n%s\n", tree.getSourceFile().getName(),
// removeThis);

/* First, collect all matches. */
SubContext context = new SubContext(state.context);
List<Description> matches = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ private static void collectRuleIdentifiers(
}
}

// XXX: Decompose `RefasterRule`s such that each rule has exactly one `@BeforeTemplate`.
// XXX: Consider decomposing `RefasterRule`s such that each rule has exactly one
// `@BeforeTemplate`.
private static ImmutableSet<ImmutableSet<String>> extractRuleIdentifiers(
RefasterRule<?, ?> refasterRule) {
ImmutableSet.Builder<ImmutableSet<String>> results = ImmutableSet.builder();
Expand Down Expand Up @@ -285,13 +286,7 @@ private static String treeKindToString(Tree.Kind kind) {

private static final class RefasterIntrospection {
private static final String UCLASS_IDENT_FQCN = "com.google.errorprone.refaster.UClassIdent";
// XXX: Probably there is a better way to fix this... For a few BeforeTemplates like
// `ImmutableMapBuilder` the algorithm wouldn't match so created this fix for now. About 10
// templates would always match.
private static final String AUTO_VALUE_UCLASS_IDENT_FQCN =
"com.google.errorprone.refaster.AutoValue_UClassIdent";
private static final Class<?> UCLASS_IDENT = getClass(UCLASS_IDENT_FQCN);
private static final Class<?> UCLASS_AUTOVALUE_IDENT = getClass(AUTO_VALUE_UCLASS_IDENT_FQCN);
private static final Method METHOD_REFASTER_RULE_BEFORE_TEMPLATES =
getMethod(RefasterRule.class, "beforeTemplates");
private static final Method METHOD_EXPRESSION_TEMPLATE_EXPRESSION =
Expand All @@ -305,7 +300,7 @@ private static final class RefasterIntrospection {
private static final Method METHOD_UANY_OF_EXPRESSIONS = getMethod(UAnyOf.class, "expressions");

static boolean isUClassIdent(IdentifierTree tree) {
return UCLASS_IDENT.equals(tree.getClass()) || UCLASS_AUTOVALUE_IDENT.equals(tree.getClass());
return UCLASS_IDENT.isAssignableFrom(tree.getClass());
}

static ImmutableList<?> getBeforeTemplates(RefasterRule<?, ?> refasterRule) {
Expand All @@ -320,12 +315,11 @@ static ImmutableList<UStatement> getTemplateStatements(BlockTemplate template) {
return invokeMethod(METHOD_BLOCK_TEMPLATE_TEMPLATE_STATEMENTS, template);
}

// Actually UClassIdent.
static IdentifierTree getClassIdent(UStaticIdent tree) {
return invokeMethod(METHOD_USTATIC_IDENT_CLASS_IDENT, tree);
}

// XXX: Make nicer. Or rename the other params.
// Arguments to this method must actually be of the package-private type `UClassIdent`.
static String getTopLevelClass(IdentifierTree uClassIdent) {
return invokeMethod(METHOD_UCLASS_IDENT_GET_TOP_LEVEL_CLASS, uClassIdent);
}
Expand Down

0 comments on commit 72f8881

Please sign in to comment.