changeset 32:0e9ef7ef1706

The Suspicious call to Collection.remove hint has been merged into NetBeans proper.
author Jan Lahoda <jlahoda@netbeans.org>
date Wed, 06 May 2009 23:26:27 +0200
parents d1c05977e165
children 24fea1d42706
files javahints/src/info/lahoda/netbeans/javahints/Bundle.properties javahints/src/info/lahoda/netbeans/javahints/CollectionRemove.java javahints/src/info/lahoda/netbeans/javahints/layer.xml javahints/test/unit/src/info/lahoda/netbeans/javahints/Bundle_test.properties javahints/test/unit/src/info/lahoda/netbeans/javahints/CollectionRemoveTest.java
diffstat 5 files changed, 0 insertions(+), 424 deletions(-) [+]
line wrap: on
line diff
--- a/javahints/src/info/lahoda/netbeans/javahints/Bundle.properties	Wed May 06 23:10:55 2009 +0200
+++ b/javahints/src/info/lahoda/netbeans/javahints/Bundle.properties	Wed May 06 23:26:27 2009 +0200
@@ -1,3 +1,1 @@
 OpenIDE-Module-Name=Lahvac's Java Hints
-HINT_SuspiciousCall=Suspicious call to {0}:\nExpected type {2}, actual type {1}
-HINT_SuspiciousCallIncompatibleTypes=Suspicious call to {0}:\nGiven object cannot contain instances of {1} (expected {2})
--- a/javahints/src/info/lahoda/netbeans/javahints/CollectionRemove.java	Wed May 06 23:10:55 2009 +0200
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,283 +0,0 @@
-package info.lahoda.netbeans.javahints;
-
-import com.sun.source.tree.ExpressionTree;
-import com.sun.source.tree.MemberSelectTree;
-import com.sun.source.tree.MethodInvocationTree;
-import com.sun.source.tree.Scope;
-import com.sun.source.tree.Tree.Kind;
-import com.sun.source.util.TreePath;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.EnumSet;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-import java.util.Map.Entry;
-import java.util.Set;
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-import javax.lang.model.element.Element;
-import javax.lang.model.element.ElementKind;
-import javax.lang.model.element.ExecutableElement;
-import javax.lang.model.element.TypeElement;
-import javax.lang.model.element.VariableElement;
-import javax.lang.model.type.DeclaredType;
-import javax.lang.model.type.ExecutableType;
-import javax.lang.model.type.TypeMirror;
-import javax.lang.model.util.ElementFilter;
-import org.netbeans.api.java.source.CompilationInfo;
-import org.netbeans.modules.editor.java.Utilities;
-import org.netbeans.modules.java.hints.spi.AbstractHint;
-import org.netbeans.spi.editor.hints.ErrorDescription;
-import org.netbeans.spi.editor.hints.ErrorDescriptionFactory;
-import org.netbeans.spi.editor.hints.Severity;
-import org.openide.util.NbBundle;
-
-/**
- *
- * @author Jan Lahoda
- */
-public class CollectionRemove extends AbstractHint {
-
-    private final AtomicBoolean cancel = new AtomicBoolean();
-    
-    public CollectionRemove() {
-        super(true, true, HintSeverity.WARNING, "collection-remove");
-    }
-
-    @Override
-    public String getDescription() {
-        return "Collection.remove";
-    }
-
-    public Set<Kind> getTreeKinds() {
-        return EnumSet.of(Kind.METHOD_INVOCATION);
-    }
-
-    public List<ErrorDescription> run(CompilationInfo info, TreePath tp) {
-        cancel.set(false);
-        MethodInvocationTree mit = (MethodInvocationTree) tp.getLeaf();
-        ExpressionTree select = mit.getMethodSelect();
-        TreePath method = new TreePath(tp, select);
-        Element el = info.getTrees().getElement(method);
-
-        if (el == null || el.getKind() != ElementKind.METHOD)
-            return null;
-
-        String simpleName = el.getSimpleName().toString();
-        ExecutableElement invoked = (ExecutableElement) el;
-        TypeElement owner = (TypeElement) invoked.getEnclosingElement();
-        Collection<MappingDefinition> mds = CHECKED_METHODS_MAP.get(simpleName);
-
-        if (mds == null) {
-            return null;
-        }
-
-        List<ErrorDescription> result = new LinkedList<ErrorDescription>();
-        
-        for (MappingDefinition md : mds) {
-            if (cancel.get())
-                return null;
-            
-            ExecutableElement toCheck = resolveMethod(info, md.checkMethod);
-
-            if (toCheck == null || (invoked != toCheck && !info.getElements().overrides(invoked, toCheck, owner))) {
-                continue;
-            }
-
-            ExecutableElement toCheckAgainst = resolveMethod(info, md.checkAgainstMethod);
-
-            if (toCheckAgainst == null) {
-                continue; //XXX: log?
-            }
-
-            DeclaredType site;
-
-            OUTER: switch (select.getKind()) {
-                case MEMBER_SELECT:
-                    site = (DeclaredType) info.getTrees().getTypeMirror(new TreePath(method, ((MemberSelectTree) select).getExpression()));
-                    break;
-                case IDENTIFIER:
-                    Scope s = info.getTrees().getScope(tp);
-
-                    while (s != null) {
-                        for (ExecutableElement ee : ElementFilter.methodsIn(info.getElements().getAllMembers(s.getEnclosingClass()))) {
-                            if (ee == toCheckAgainst || info.getElements().overrides(ee, toCheckAgainst, owner)) {
-                                site = (DeclaredType) s.getEnclosingClass().asType();
-                                break OUTER;
-                            }
-                        }
-                        s = s.getEnclosingScope();
-                    }
-
-                    site = null;
-                    break;
-                default:
-                    throw new IllegalStateException();
-            }
-
-            if (site == null) {
-                continue; //XXX: log
-            }
-
-            ExecutableType againstType = (ExecutableType) info.getTypes().asMemberOf(site, toCheckAgainst);
-
-            for (Entry<Integer,Integer> e : md.parametersMapping.entrySet()) {
-                TypeMirror actualParam = info.getTrees().getTypeMirror(new TreePath(tp, mit.getArguments().get(e.getKey())));
-                TypeMirror designedType = againstType.getParameterTypes().get(e.getValue());
-
-                if (!info.getTypes().isSubtype(actualParam,designedType)) {
-                    String warningKey;
-
-                    if (compatibleTypes(info, actualParam,designedType)) {
-                        warningKey = "HINT_SuspiciousCall";
-                    } else {
-                        warningKey = "HINT_SuspiciousCallIncompatibleTypes";
-                    }
-                    
-                    String semiFQN = md.checkMethod.substring(0, md.checkMethod.indexOf('('));
-
-                    String warning = NbBundle.getMessage(CollectionRemove.class,
-                                                         warningKey,
-                                                         semiFQN,
-                                                         Utilities.getTypeName(actualParam, false),
-                                                         Utilities.getTypeName(designedType, false));
-
-                    int start = (int) info.getTrees().getSourcePositions().getStartPosition(info.getCompilationUnit(), mit);
-                    int end   = (int) info.getTrees().getSourcePositions().getEndPosition(info.getCompilationUnit(), mit);
-
-                    result.add(ErrorDescriptionFactory.createErrorDescription(Severity.WARNING, warning, info.getFileObject(), start, end));
-                }
-            }
-        }
-
-        return result;
-    }
-
-    private static boolean compatibleTypes(CompilationInfo info, TypeMirror type1, TypeMirror type2) {
-        type1 = info.getTypes().erasure(type1);
-        type2 = info.getTypes().erasure(type2);
-
-        return info.getTypeUtilities().isCastable(type1, type2);
-    }
-
-    private static final Pattern SPLIT = Pattern.compile("(.+)\\.([^.]+)\\((.*)\\)");
-    
-    private static ExecutableElement resolveMethod(CompilationInfo info, String name) {
-        Matcher m = SPLIT.matcher(name);
-
-        if (!m.matches()) {
-            throw new IllegalArgumentException();
-        }
-
-        String className = m.group(1);
-        String methodName = m.group(2);
-        String paramsSpec = m.group(3);
-
-        TypeElement te = info.getElements().getTypeElement(className);
-
-        if (te == null) {
-            return null;
-        }
-
-        String[] paramList = paramsSpec.split(",");
-        List<TypeMirror> params = new LinkedList<TypeMirror>();
-
-        TypeElement topLevel = info.getTopLevelElements().get(0);
-
-        for (String t : paramList) {
-            params.add(info.getTreeUtilities().parseType(t, topLevel));
-        }
-
-        for (ExecutableElement ee : ElementFilter.methodsIn(te.getEnclosedElements())) {
-            if (!methodName.equals(ee.getSimpleName().toString()) || ee.getParameters().size() != params.size()) {
-                continue;
-            }
-            
-            Iterator<TypeMirror> designed = params.iterator();
-            boolean found = true;
-
-            for (VariableElement param : ee.getParameters()) {
-                if (!info.getTypes().isSameType(info.getTypes().erasure(param.asType()), designed.next())) {
-                    found = false;
-                    break;
-                }
-            }
-
-            if (found) {
-                return ee;
-            }
-        }
-        
-        return null;
-    }
-
-    public String getId() {
-        return CollectionRemove.class.getName();
-    }
-
-    public String getDisplayName() {
-        return "Collection.remove";
-    }
-
-    public void cancel() {
-        cancel.set(true);
-    }
-
-    private static final Iterable<MappingDefinition> MAPPINGS = Arrays.asList(
-            new MappingDefinition("java.util.Collection.remove(java.lang.Object)", "java.util.Collection.add(java.lang.Object)", 0, 0),
-            new MappingDefinition("java.util.Collection.contains(java.lang.Object)", "java.util.Collection.add(java.lang.Object)", 0, 0),
-            new MappingDefinition("java.util.Map.containsKey(java.lang.Object)", "java.util.Map.put(java.lang.Object, java.lang.Object)", 0, 0),
-            new MappingDefinition("java.util.Map.get(java.lang.Object)", "java.util.Map.put(java.lang.Object, java.lang.Object)", 0, 0),
-            new MappingDefinition("java.util.Map.remove(java.lang.Object)", "java.util.Map.put(java.lang.Object, java.lang.Object)", 0, 0),
-            new MappingDefinition("java.util.Map.containsValue(java.lang.Object)", "java.util.Map.put(java.lang.Object, java.lang.Object)", 0, 1)
-    );
-
-    private static final Map<String, Collection<MappingDefinition>> CHECKED_METHODS_MAP;
-
-    static {
-        CHECKED_METHODS_MAP = new HashMap<String, Collection<MappingDefinition>>();
-        
-        for (MappingDefinition d : MAPPINGS) {
-            Matcher m = SPLIT.matcher(d.checkMethod);
-
-            if (!m.matches()) {
-                throw new IllegalArgumentException();
-            }
-
-            String methodName = m.group(2);
-            Collection<MappingDefinition> c = CHECKED_METHODS_MAP.get(methodName);
-
-            if (c == null) {
-                CHECKED_METHODS_MAP.put(methodName, c = new LinkedList<MappingDefinition>());
-            }
-
-            c.add(d);
-        }
-    }
-
-    private static final class MappingDefinition {
-
-        private final String checkMethod;
-        private final String checkAgainstMethod;
-        private final Map<Integer, Integer> parametersMapping;
-
-        public MappingDefinition(String checkMethod, String checkAgainstMethod, int... parametersMapping) {
-            this.checkMethod = checkMethod;
-            this.checkAgainstMethod = checkAgainstMethod;
-
-            assert parametersMapping.length % 2 == 0;
-            
-            this.parametersMapping = new HashMap<Integer, Integer>();
-
-            for (int cntr = 0; cntr < parametersMapping.length; cntr += 2) {
-                this.parametersMapping.put(parametersMapping[cntr], parametersMapping[cntr + 1]);
-            }
-        }
-
-    }
-
-}
--- a/javahints/src/info/lahoda/netbeans/javahints/layer.xml	Wed May 06 23:10:55 2009 +0200
+++ b/javahints/src/info/lahoda/netbeans/javahints/layer.xml	Wed May 06 23:26:27 2009 +0200
@@ -5,7 +5,6 @@
         <folder name="rules">
             <folder name="hints">
                 <folder name="experimental">
-                    <file name="info-lahoda-netbeans-javahints-CollectionRemove.instance"/>
                     <file name="org-netbeans-modules-javahints-NPECheck.instance"/>
                 </folder>
             </folder>
--- a/javahints/test/unit/src/info/lahoda/netbeans/javahints/Bundle_test.properties	Wed May 06 23:10:55 2009 +0200
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,2 +0,0 @@
-HINT_SuspiciousCall=SC: {0}, {1}, {2}
-HINT_SuspiciousCallIncompatibleTypes=SCIT: {0}, {1}, {2}
--- a/javahints/test/unit/src/info/lahoda/netbeans/javahints/CollectionRemoveTest.java	Wed May 06 23:10:55 2009 +0200
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,136 +0,0 @@
-package info.lahoda.netbeans.javahints;
-
-import com.sun.source.tree.Tree.Kind;
-import com.sun.source.util.TreePath;
-import java.util.List;
-import org.netbeans.api.java.source.CompilationInfo;
-import org.netbeans.modules.java.hints.infrastructure.TreeRuleTestBase;
-import org.netbeans.spi.editor.hints.ErrorDescription;
-import org.openide.util.NbBundle;
-
-/**
- *
- * @author Jan Lahoda
- */
-public class CollectionRemoveTest extends TreeRuleTestBase {
-
-    public CollectionRemoveTest(String name) {
-        super(name);
-    }
-
-    public void testSimple1() throws Exception {
-        performAnalysisTest("test/Test.java",
-                            "package test;" +
-                            "public class Test {" +
-                            "    private void test () {" +
-                            "        java.util.List<String> l = null;" +
-                            "        l.rem|ove(new Object());" +
-                            "    }" +
-                            "}",
-                            "0:106-0:128:warning:SC: java.util.Collection.remove, Object, String");
-    }
-
-    public void testSimple2() throws Exception {
-        performAnalysisTest("test/Test.java",
-                            "package test;" +
-                            "public class Test {" +
-                            "    private void test () {" +
-                            "        java.util.List<String> l = null;" +
-                            "        l.con|tains(new Object());" +
-                            "    }" +
-                            "}",
-                            "0:106-0:130:warning:SC: java.util.Collection.contains, Object, String");
-    }
-
-    public void testSimple3() throws Exception {
-        performAnalysisTest("test/Test.java",
-                            "package test;" +
-                            "public class Test {" +
-                            "    private void test () {" +
-                            "        java.util.List<String> l = null;" +
-                            "        l.rem|ove(Integer.valueOf(1));" +
-                            "    }" +
-                            "}",
-                            "0:106-0:134:warning:SCIT: java.util.Collection.remove, Integer, String");
-    }
-
-    public void testSimple4() throws Exception {
-        performAnalysisTest("test/Test.java",
-                            "package test;" +
-                            "public class Test {" +
-                            "    private void test () {" +
-                            "        java.util.List<String> l = null;" +
-                            "        l.con|tains(Integer.valueOf(1));" +
-                            "    }" +
-                            "}",
-                            "0:106-0:136:warning:SCIT: java.util.Collection.contains, Integer, String");
-    }
-
-    public void testSimple5() throws Exception {
-        performAnalysisTest("test/Test.java",
-                            "package test;" +
-                            "public class Test {" +
-                            "    private void test () {" +
-                            "        java.util.Map<String, Number> l = null;" +
-                            "        l.con|tainsKey(Integer.valueOf(1));" +
-                            "    }" +
-                            "}",
-                            "0:113-0:146:warning:SCIT: java.util.Map.containsKey, Integer, String");
-    }
-
-    public void testSimple6() throws Exception {
-        performAnalysisTest("test/Test.java",
-                            "package test;" +
-                            "public class Test {" +
-                            "    private void test () {" +
-                            "        java.util.Map<String, Number> l = null;" +
-                            "        l.con|tainsValue(\"\");" +
-                            "    }" +
-                            "}",
-                            "0:113-0:132:warning:SCIT: java.util.Map.containsValue, String, Number");
-    }
-
-    public void testExtends1() throws Exception {
-        performAnalysisTest("test/Test.java",
-                            "package test;" +
-                            "public class Test extends java.util.LinkedList<String> {" +
-                            "    private void test () {" +
-                            "        re|move(new Object());" +
-                            "    }" +
-                            "}",
-                            "0:103-0:123:warning:SC: java.util.Collection.remove, Object, String");
-    }
-
-    public void testExtends2() throws Exception {
-        performAnalysisTest("test/Test.java",
-                            "package test;" +
-                            "public class Test extends java.util.LinkedList<String> {" +
-                            "    private void test () {" +
-                            "        new Runnable() {" +
-                            "            public void run() {" +
-                            "                re|move(new Object());" +
-                            "            }" +
-                            "        }" +
-                            "    }" +
-                            "}",
-                            "0:166-0:186:warning:SC: java.util.Collection.remove, Object, String");
-    }
-
-    @Override
-    protected List<ErrorDescription> computeErrors(CompilationInfo info, TreePath tp) {
-        while (tp != null) {
-            if (tp.getLeaf().getKind() != Kind.METHOD_INVOCATION) {
-                tp = tp.getParentPath();
-                continue;
-            }
-            return new CollectionRemove().run(info, tp);
-        }
-
-        return null;
-    }
-
-    static {
-        NbBundle.setBranding("test");
-    }
-
-}