Skip to content

Commit

Permalink
RELNOTES: Add a new Error Prone check to warn when a type parameter s…
Browse files Browse the repository at this point in the history
…hadows another type in scope.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=199379629
  • Loading branch information
bennostein authored and cushon committed Jun 12, 2018
1 parent afbddb0 commit 0aa9d3f
Show file tree
Hide file tree
Showing 5 changed files with 523 additions and 3 deletions.
@@ -0,0 +1,173 @@
/*
* Copyright 2018 The Error Prone Authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.errorprone.bugpatterns;

import static com.google.errorprone.BugPattern.Category.JDK;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.ProvidesFix;
import com.google.errorprone.BugPattern.StandardTags;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.TypeParameterTree;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symbol.ClassSymbol;
import com.sun.tools.javac.code.Symbol.TypeVariableSymbol;
import com.sun.tools.javac.comp.AttrContext;
import com.sun.tools.javac.comp.Enter;
import com.sun.tools.javac.comp.Env;
import com.sun.tools.javac.tree.JCTree.Tag;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;

/**
* Warns when a type parameter shadows another type name in scope.
*
* @author bennostein@google.com (Benno Stein)
*/
@BugPattern(
name = "TypeNameShadowing",
summary = "Type parameter declaration shadows another named type",
category = JDK,
severity = WARNING,
tags = StandardTags.STYLE,
providesFix = ProvidesFix.REQUIRES_HUMAN_ATTENTION)
public class TypeNameShadowing extends BugChecker implements MethodTreeMatcher, ClassTreeMatcher {

@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
if (tree.getTypeParameters().isEmpty()) {
return Description.NO_MATCH;
}
return findShadowedTypes(tree, tree.getTypeParameters(), state);
}

@Override
public Description matchClass(ClassTree tree, VisitorState state) {
if (tree.getTypeParameters().isEmpty()) {
return Description.NO_MATCH;
}
return findShadowedTypes(tree, tree.getTypeParameters(), state);
}

/**
* Returns a list of type symbols in the scope enclosing {@code env} guaranteeing that, when two
* symbols share a simple name, the shadower precedes the shadow-ee
*/
private static Iterable<Symbol> typesInEnclosingScope(Env<AttrContext> env) {
// Collect all visible type names declared in this source file by ascending lexical scopes
// and excluding TypeVariableSymbols (otherwise, every type parameter spuriously shadows itself)
Iterable<Symbol> localSymbolsInScope =
Streams.stream(env)
.map(
ctx ->
ctx.tree.getTag() == Tag.CLASSDEF
? ((ClassSymbol) ASTHelpers.getSymbol(ctx.tree)).members().getSymbols()
: ctx.info.getLocalElements())
.flatMap(
symbols ->
Streams.stream(symbols).filter(sym -> !(sym instanceof TypeVariableSymbol)))
.collect(ImmutableList.toImmutableList());

// Concatenate with all visible type names declared in other source files:
return Iterables.concat(
localSymbolsInScope, // Local symbols
env.toplevel.namedImportScope.getSymbols(), // Explicitly named imports
env.toplevel.starImportScope.getSymbols(), // Wildcard imports
env.toplevel.packge.members().getSymbols()); // Siblings in class hierarchy
}

/** Iterate through a list of type parameters, looking for type names shadowed by any of them. */
private Description findShadowedTypes(
Tree tree, List<? extends TypeParameterTree> typeParameters, VisitorState state) {

Env<AttrContext> env =
Enter.instance(state.context)
.getEnv(ASTHelpers.getSymbol(state.findEnclosing(ClassTree.class)));
Iterable<Symbol> enclosingTypes = typesInEnclosingScope(env);

List<Symbol> shadowedTypes =
typeParameters
.stream()
.map(
param ->
Iterables.tryFind(
enclosingTypes,
sym ->
sym.getSimpleName()
.equals(ASTHelpers.getType(param).tsym.getSimpleName()))
.orNull())
.filter(Objects::nonNull)
.collect(ImmutableList.toImmutableList());

if (shadowedTypes.isEmpty()) {
return Description.NO_MATCH;
}

Description.Builder descBuilder = buildDescription(tree);

descBuilder.setMessage(buildMessage(shadowedTypes));

Set<String> visibleNames =
Streams.stream(Iterables.concat(env.info.getLocalElements(), enclosingTypes))
.map(sym -> sym.getSimpleName().toString())
.collect(ImmutableSet.toImmutableSet());

SuggestedFix.Builder fixBuilder = SuggestedFix.builder();
shadowedTypes
.stream()
.filter(tv -> TypeParameterNaming.matchesTypeParameterNamingScheme(tv.name))
.map(
tv ->
TypeParameterShadowing.renameTypeVariable(
TypeParameterShadowing.typeParameterInList(typeParameters, tv),
tree,
TypeParameterShadowing.replacementTypeVarName(tv.name, visibleNames),
state))
.forEach(fixBuilder::merge);

descBuilder.addFix(fixBuilder.build());
return descBuilder.build();
}

private static String buildMessage(List<Symbol> shadowedTypes) {
return "Found type parameters shadowing other declared types:\n\t"
+ shadowedTypes
.stream()
.map(
sym ->
"Type parameter "
+ sym.getSimpleName()
+ " shadows visible type "
+ sym.flatName())
.collect(Collectors.joining("\n\t"));
}
}
Expand Up @@ -129,19 +129,22 @@ private Description findDuplicatesOf(
return descriptionBuilder.build(); return descriptionBuilder.build();
} }


private TypeParameterTree typeParameterInList( // Package-private for TypeNameShadowing
List<? extends TypeParameterTree> typeParameters, TypeVariableSymbol v) { static TypeParameterTree typeParameterInList(
List<? extends TypeParameterTree> typeParameters, Symbol v) {
return typeParameters return typeParameters
.stream() .stream()
.filter(t -> t.getName().contentEquals(v.name)) .filter(t -> t.getName().contentEquals(v.name))
.collect(MoreCollectors.onlyElement()); .collect(MoreCollectors.onlyElement());
} }


private static final Pattern TRAILING_DIGIT_EXTRACTOR = Pattern.compile("^(.*?)(\\d+)$"); private static final Pattern TRAILING_DIGIT_EXTRACTOR = Pattern.compile("^(.*?)(\\d+)$");

// T -> T2 // T -> T2
// T2 -> T3 // T2 -> T3
// T -> T4 (if T2 and T3 already exist) // T -> T4 (if T2 and T3 already exist)
private String replacementTypeVarName(Name name, Set<String> superTypeVars) { // Package-private for TypeNameShadowing
static String replacementTypeVarName(Name name, Set<String> superTypeVars) {
String baseName = name.toString(); String baseName = name.toString();
int typeVarNum = 2; int typeVarNum = 2;


Expand Down
Expand Up @@ -224,6 +224,7 @@
import com.google.errorprone.bugpatterns.TruthConstantAsserts; import com.google.errorprone.bugpatterns.TruthConstantAsserts;
import com.google.errorprone.bugpatterns.TruthSelfEquals; import com.google.errorprone.bugpatterns.TruthSelfEquals;
import com.google.errorprone.bugpatterns.TryFailThrowable; import com.google.errorprone.bugpatterns.TryFailThrowable;
import com.google.errorprone.bugpatterns.TypeNameShadowing;
import com.google.errorprone.bugpatterns.TypeParameterNaming; import com.google.errorprone.bugpatterns.TypeParameterNaming;
import com.google.errorprone.bugpatterns.TypeParameterQualifier; import com.google.errorprone.bugpatterns.TypeParameterQualifier;
import com.google.errorprone.bugpatterns.TypeParameterShadowing; import com.google.errorprone.bugpatterns.TypeParameterShadowing;
Expand Down Expand Up @@ -563,6 +564,7 @@ public static ScannerSupplier errorChecks() {
TruthAssertExpected.class, TruthAssertExpected.class,
TruthConstantAsserts.class, TruthConstantAsserts.class,
TruthIncompatibleType.class, TruthIncompatibleType.class,
TypeNameShadowing.class,
TypeParameterShadowing.class, TypeParameterShadowing.class,
TypeParameterUnusedInFormals.class, TypeParameterUnusedInFormals.class,
UnsafeFinalization.class, UnsafeFinalization.class,
Expand Down

0 comments on commit 0aa9d3f

Please sign in to comment.