lahvacs
changeset 32:0e9ef7ef1706
The Suspicious call to Collection.remove hint has been merged into NetBeans proper.
| author | Jan Lahoda <jlahoda@netbeans.org> |
|---|---|
| date | Wed May 06 23:26:27 2009 +0200 (3 years ago) |
| 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 |
line diff
1.1 --- a/javahints/src/info/lahoda/netbeans/javahints/Bundle.properties Wed May 06 23:10:55 2009 +0200
1.2 +++ b/javahints/src/info/lahoda/netbeans/javahints/Bundle.properties Wed May 06 23:26:27 2009 +0200
1.3 @@ -1,3 +1,1 @@
1.4 OpenIDE-Module-Name=Lahvac's Java Hints
1.5 -HINT_SuspiciousCall=Suspicious call to {0}:\nExpected type {2}, actual type {1}
1.6 -HINT_SuspiciousCallIncompatibleTypes=Suspicious call to {0}:\nGiven object cannot contain instances of {1} (expected {2})
2.1 --- a/javahints/src/info/lahoda/netbeans/javahints/CollectionRemove.java Wed May 06 23:10:55 2009 +0200
2.2 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000
2.3 @@ -1,283 +0,0 @@
2.4 -package info.lahoda.netbeans.javahints;
2.5 -
2.6 -import com.sun.source.tree.ExpressionTree;
2.7 -import com.sun.source.tree.MemberSelectTree;
2.8 -import com.sun.source.tree.MethodInvocationTree;
2.9 -import com.sun.source.tree.Scope;
2.10 -import com.sun.source.tree.Tree.Kind;
2.11 -import com.sun.source.util.TreePath;
2.12 -import java.util.Arrays;
2.13 -import java.util.Collection;
2.14 -import java.util.EnumSet;
2.15 -import java.util.HashMap;
2.16 -import java.util.Iterator;
2.17 -import java.util.LinkedList;
2.18 -import java.util.List;
2.19 -import java.util.Map;
2.20 -import java.util.Map.Entry;
2.21 -import java.util.Set;
2.22 -import java.util.concurrent.atomic.AtomicBoolean;
2.23 -import java.util.regex.Matcher;
2.24 -import java.util.regex.Pattern;
2.25 -import javax.lang.model.element.Element;
2.26 -import javax.lang.model.element.ElementKind;
2.27 -import javax.lang.model.element.ExecutableElement;
2.28 -import javax.lang.model.element.TypeElement;
2.29 -import javax.lang.model.element.VariableElement;
2.30 -import javax.lang.model.type.DeclaredType;
2.31 -import javax.lang.model.type.ExecutableType;
2.32 -import javax.lang.model.type.TypeMirror;
2.33 -import javax.lang.model.util.ElementFilter;
2.34 -import org.netbeans.api.java.source.CompilationInfo;
2.35 -import org.netbeans.modules.editor.java.Utilities;
2.36 -import org.netbeans.modules.java.hints.spi.AbstractHint;
2.37 -import org.netbeans.spi.editor.hints.ErrorDescription;
2.38 -import org.netbeans.spi.editor.hints.ErrorDescriptionFactory;
2.39 -import org.netbeans.spi.editor.hints.Severity;
2.40 -import org.openide.util.NbBundle;
2.41 -
2.42 -/**
2.43 - *
2.44 - * @author Jan Lahoda
2.45 - */
2.46 -public class CollectionRemove extends AbstractHint {
2.47 -
2.48 - private final AtomicBoolean cancel = new AtomicBoolean();
2.49 -
2.50 - public CollectionRemove() {
2.51 - super(true, true, HintSeverity.WARNING, "collection-remove");
2.52 - }
2.53 -
2.54 - @Override
2.55 - public String getDescription() {
2.56 - return "Collection.remove";
2.57 - }
2.58 -
2.59 - public Set<Kind> getTreeKinds() {
2.60 - return EnumSet.of(Kind.METHOD_INVOCATION);
2.61 - }
2.62 -
2.63 - public List<ErrorDescription> run(CompilationInfo info, TreePath tp) {
2.64 - cancel.set(false);
2.65 - MethodInvocationTree mit = (MethodInvocationTree) tp.getLeaf();
2.66 - ExpressionTree select = mit.getMethodSelect();
2.67 - TreePath method = new TreePath(tp, select);
2.68 - Element el = info.getTrees().getElement(method);
2.69 -
2.70 - if (el == null || el.getKind() != ElementKind.METHOD)
2.71 - return null;
2.72 -
2.73 - String simpleName = el.getSimpleName().toString();
2.74 - ExecutableElement invoked = (ExecutableElement) el;
2.75 - TypeElement owner = (TypeElement) invoked.getEnclosingElement();
2.76 - Collection<MappingDefinition> mds = CHECKED_METHODS_MAP.get(simpleName);
2.77 -
2.78 - if (mds == null) {
2.79 - return null;
2.80 - }
2.81 -
2.82 - List<ErrorDescription> result = new LinkedList<ErrorDescription>();
2.83 -
2.84 - for (MappingDefinition md : mds) {
2.85 - if (cancel.get())
2.86 - return null;
2.87 -
2.88 - ExecutableElement toCheck = resolveMethod(info, md.checkMethod);
2.89 -
2.90 - if (toCheck == null || (invoked != toCheck && !info.getElements().overrides(invoked, toCheck, owner))) {
2.91 - continue;
2.92 - }
2.93 -
2.94 - ExecutableElement toCheckAgainst = resolveMethod(info, md.checkAgainstMethod);
2.95 -
2.96 - if (toCheckAgainst == null) {
2.97 - continue; //XXX: log?
2.98 - }
2.99 -
2.100 - DeclaredType site;
2.101 -
2.102 - OUTER: switch (select.getKind()) {
2.103 - case MEMBER_SELECT:
2.104 - site = (DeclaredType) info.getTrees().getTypeMirror(new TreePath(method, ((MemberSelectTree) select).getExpression()));
2.105 - break;
2.106 - case IDENTIFIER:
2.107 - Scope s = info.getTrees().getScope(tp);
2.108 -
2.109 - while (s != null) {
2.110 - for (ExecutableElement ee : ElementFilter.methodsIn(info.getElements().getAllMembers(s.getEnclosingClass()))) {
2.111 - if (ee == toCheckAgainst || info.getElements().overrides(ee, toCheckAgainst, owner)) {
2.112 - site = (DeclaredType) s.getEnclosingClass().asType();
2.113 - break OUTER;
2.114 - }
2.115 - }
2.116 - s = s.getEnclosingScope();
2.117 - }
2.118 -
2.119 - site = null;
2.120 - break;
2.121 - default:
2.122 - throw new IllegalStateException();
2.123 - }
2.124 -
2.125 - if (site == null) {
2.126 - continue; //XXX: log
2.127 - }
2.128 -
2.129 - ExecutableType againstType = (ExecutableType) info.getTypes().asMemberOf(site, toCheckAgainst);
2.130 -
2.131 - for (Entry<Integer,Integer> e : md.parametersMapping.entrySet()) {
2.132 - TypeMirror actualParam = info.getTrees().getTypeMirror(new TreePath(tp, mit.getArguments().get(e.getKey())));
2.133 - TypeMirror designedType = againstType.getParameterTypes().get(e.getValue());
2.134 -
2.135 - if (!info.getTypes().isSubtype(actualParam,designedType)) {
2.136 - String warningKey;
2.137 -
2.138 - if (compatibleTypes(info, actualParam,designedType)) {
2.139 - warningKey = "HINT_SuspiciousCall";
2.140 - } else {
2.141 - warningKey = "HINT_SuspiciousCallIncompatibleTypes";
2.142 - }
2.143 -
2.144 - String semiFQN = md.checkMethod.substring(0, md.checkMethod.indexOf('('));
2.145 -
2.146 - String warning = NbBundle.getMessage(CollectionRemove.class,
2.147 - warningKey,
2.148 - semiFQN,
2.149 - Utilities.getTypeName(actualParam, false),
2.150 - Utilities.getTypeName(designedType, false));
2.151 -
2.152 - int start = (int) info.getTrees().getSourcePositions().getStartPosition(info.getCompilationUnit(), mit);
2.153 - int end = (int) info.getTrees().getSourcePositions().getEndPosition(info.getCompilationUnit(), mit);
2.154 -
2.155 - result.add(ErrorDescriptionFactory.createErrorDescription(Severity.WARNING, warning, info.getFileObject(), start, end));
2.156 - }
2.157 - }
2.158 - }
2.159 -
2.160 - return result;
2.161 - }
2.162 -
2.163 - private static boolean compatibleTypes(CompilationInfo info, TypeMirror type1, TypeMirror type2) {
2.164 - type1 = info.getTypes().erasure(type1);
2.165 - type2 = info.getTypes().erasure(type2);
2.166 -
2.167 - return info.getTypeUtilities().isCastable(type1, type2);
2.168 - }
2.169 -
2.170 - private static final Pattern SPLIT = Pattern.compile("(.+)\\.([^.]+)\\((.*)\\)");
2.171 -
2.172 - private static ExecutableElement resolveMethod(CompilationInfo info, String name) {
2.173 - Matcher m = SPLIT.matcher(name);
2.174 -
2.175 - if (!m.matches()) {
2.176 - throw new IllegalArgumentException();
2.177 - }
2.178 -
2.179 - String className = m.group(1);
2.180 - String methodName = m.group(2);
2.181 - String paramsSpec = m.group(3);
2.182 -
2.183 - TypeElement te = info.getElements().getTypeElement(className);
2.184 -
2.185 - if (te == null) {
2.186 - return null;
2.187 - }
2.188 -
2.189 - String[] paramList = paramsSpec.split(",");
2.190 - List<TypeMirror> params = new LinkedList<TypeMirror>();
2.191 -
2.192 - TypeElement topLevel = info.getTopLevelElements().get(0);
2.193 -
2.194 - for (String t : paramList) {
2.195 - params.add(info.getTreeUtilities().parseType(t, topLevel));
2.196 - }
2.197 -
2.198 - for (ExecutableElement ee : ElementFilter.methodsIn(te.getEnclosedElements())) {
2.199 - if (!methodName.equals(ee.getSimpleName().toString()) || ee.getParameters().size() != params.size()) {
2.200 - continue;
2.201 - }
2.202 -
2.203 - Iterator<TypeMirror> designed = params.iterator();
2.204 - boolean found = true;
2.205 -
2.206 - for (VariableElement param : ee.getParameters()) {
2.207 - if (!info.getTypes().isSameType(info.getTypes().erasure(param.asType()), designed.next())) {
2.208 - found = false;
2.209 - break;
2.210 - }
2.211 - }
2.212 -
2.213 - if (found) {
2.214 - return ee;
2.215 - }
2.216 - }
2.217 -
2.218 - return null;
2.219 - }
2.220 -
2.221 - public String getId() {
2.222 - return CollectionRemove.class.getName();
2.223 - }
2.224 -
2.225 - public String getDisplayName() {
2.226 - return "Collection.remove";
2.227 - }
2.228 -
2.229 - public void cancel() {
2.230 - cancel.set(true);
2.231 - }
2.232 -
2.233 - private static final Iterable<MappingDefinition> MAPPINGS = Arrays.asList(
2.234 - new MappingDefinition("java.util.Collection.remove(java.lang.Object)", "java.util.Collection.add(java.lang.Object)", 0, 0),
2.235 - new MappingDefinition("java.util.Collection.contains(java.lang.Object)", "java.util.Collection.add(java.lang.Object)", 0, 0),
2.236 - new MappingDefinition("java.util.Map.containsKey(java.lang.Object)", "java.util.Map.put(java.lang.Object, java.lang.Object)", 0, 0),
2.237 - new MappingDefinition("java.util.Map.get(java.lang.Object)", "java.util.Map.put(java.lang.Object, java.lang.Object)", 0, 0),
2.238 - new MappingDefinition("java.util.Map.remove(java.lang.Object)", "java.util.Map.put(java.lang.Object, java.lang.Object)", 0, 0),
2.239 - new MappingDefinition("java.util.Map.containsValue(java.lang.Object)", "java.util.Map.put(java.lang.Object, java.lang.Object)", 0, 1)
2.240 - );
2.241 -
2.242 - private static final Map<String, Collection<MappingDefinition>> CHECKED_METHODS_MAP;
2.243 -
2.244 - static {
2.245 - CHECKED_METHODS_MAP = new HashMap<String, Collection<MappingDefinition>>();
2.246 -
2.247 - for (MappingDefinition d : MAPPINGS) {
2.248 - Matcher m = SPLIT.matcher(d.checkMethod);
2.249 -
2.250 - if (!m.matches()) {
2.251 - throw new IllegalArgumentException();
2.252 - }
2.253 -
2.254 - String methodName = m.group(2);
2.255 - Collection<MappingDefinition> c = CHECKED_METHODS_MAP.get(methodName);
2.256 -
2.257 - if (c == null) {
2.258 - CHECKED_METHODS_MAP.put(methodName, c = new LinkedList<MappingDefinition>());
2.259 - }
2.260 -
2.261 - c.add(d);
2.262 - }
2.263 - }
2.264 -
2.265 - private static final class MappingDefinition {
2.266 -
2.267 - private final String checkMethod;
2.268 - private final String checkAgainstMethod;
2.269 - private final Map<Integer, Integer> parametersMapping;
2.270 -
2.271 - public MappingDefinition(String checkMethod, String checkAgainstMethod, int... parametersMapping) {
2.272 - this.checkMethod = checkMethod;
2.273 - this.checkAgainstMethod = checkAgainstMethod;
2.274 -
2.275 - assert parametersMapping.length % 2 == 0;
2.276 -
2.277 - this.parametersMapping = new HashMap<Integer, Integer>();
2.278 -
2.279 - for (int cntr = 0; cntr < parametersMapping.length; cntr += 2) {
2.280 - this.parametersMapping.put(parametersMapping[cntr], parametersMapping[cntr + 1]);
2.281 - }
2.282 - }
2.283 -
2.284 - }
2.285 -
2.286 -}
3.1 --- a/javahints/src/info/lahoda/netbeans/javahints/layer.xml Wed May 06 23:10:55 2009 +0200
3.2 +++ b/javahints/src/info/lahoda/netbeans/javahints/layer.xml Wed May 06 23:26:27 2009 +0200
3.3 @@ -5,7 +5,6 @@
3.4 <folder name="rules">
3.5 <folder name="hints">
3.6 <folder name="experimental">
3.7 - <file name="info-lahoda-netbeans-javahints-CollectionRemove.instance"/>
3.8 <file name="org-netbeans-modules-javahints-NPECheck.instance"/>
3.9 </folder>
3.10 </folder>
4.1 --- a/javahints/test/unit/src/info/lahoda/netbeans/javahints/Bundle_test.properties Wed May 06 23:10:55 2009 +0200
4.2 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000
4.3 @@ -1,2 +0,0 @@
4.4 -HINT_SuspiciousCall=SC: {0}, {1}, {2}
4.5 -HINT_SuspiciousCallIncompatibleTypes=SCIT: {0}, {1}, {2}
5.1 --- a/javahints/test/unit/src/info/lahoda/netbeans/javahints/CollectionRemoveTest.java Wed May 06 23:10:55 2009 +0200
5.2 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000
5.3 @@ -1,136 +0,0 @@
5.4 -package info.lahoda.netbeans.javahints;
5.5 -
5.6 -import com.sun.source.tree.Tree.Kind;
5.7 -import com.sun.source.util.TreePath;
5.8 -import java.util.List;
5.9 -import org.netbeans.api.java.source.CompilationInfo;
5.10 -import org.netbeans.modules.java.hints.infrastructure.TreeRuleTestBase;
5.11 -import org.netbeans.spi.editor.hints.ErrorDescription;
5.12 -import org.openide.util.NbBundle;
5.13 -
5.14 -/**
5.15 - *
5.16 - * @author Jan Lahoda
5.17 - */
5.18 -public class CollectionRemoveTest extends TreeRuleTestBase {
5.19 -
5.20 - public CollectionRemoveTest(String name) {
5.21 - super(name);
5.22 - }
5.23 -
5.24 - public void testSimple1() throws Exception {
5.25 - performAnalysisTest("test/Test.java",
5.26 - "package test;" +
5.27 - "public class Test {" +
5.28 - " private void test () {" +
5.29 - " java.util.List<String> l = null;" +
5.30 - " l.rem|ove(new Object());" +
5.31 - " }" +
5.32 - "}",
5.33 - "0:106-0:128:warning:SC: java.util.Collection.remove, Object, String");
5.34 - }
5.35 -
5.36 - public void testSimple2() throws Exception {
5.37 - performAnalysisTest("test/Test.java",
5.38 - "package test;" +
5.39 - "public class Test {" +
5.40 - " private void test () {" +
5.41 - " java.util.List<String> l = null;" +
5.42 - " l.con|tains(new Object());" +
5.43 - " }" +
5.44 - "}",
5.45 - "0:106-0:130:warning:SC: java.util.Collection.contains, Object, String");
5.46 - }
5.47 -
5.48 - public void testSimple3() throws Exception {
5.49 - performAnalysisTest("test/Test.java",
5.50 - "package test;" +
5.51 - "public class Test {" +
5.52 - " private void test () {" +
5.53 - " java.util.List<String> l = null;" +
5.54 - " l.rem|ove(Integer.valueOf(1));" +
5.55 - " }" +
5.56 - "}",
5.57 - "0:106-0:134:warning:SCIT: java.util.Collection.remove, Integer, String");
5.58 - }
5.59 -
5.60 - public void testSimple4() throws Exception {
5.61 - performAnalysisTest("test/Test.java",
5.62 - "package test;" +
5.63 - "public class Test {" +
5.64 - " private void test () {" +
5.65 - " java.util.List<String> l = null;" +
5.66 - " l.con|tains(Integer.valueOf(1));" +
5.67 - " }" +
5.68 - "}",
5.69 - "0:106-0:136:warning:SCIT: java.util.Collection.contains, Integer, String");
5.70 - }
5.71 -
5.72 - public void testSimple5() throws Exception {
5.73 - performAnalysisTest("test/Test.java",
5.74 - "package test;" +
5.75 - "public class Test {" +
5.76 - " private void test () {" +
5.77 - " java.util.Map<String, Number> l = null;" +
5.78 - " l.con|tainsKey(Integer.valueOf(1));" +
5.79 - " }" +
5.80 - "}",
5.81 - "0:113-0:146:warning:SCIT: java.util.Map.containsKey, Integer, String");
5.82 - }
5.83 -
5.84 - public void testSimple6() throws Exception {
5.85 - performAnalysisTest("test/Test.java",
5.86 - "package test;" +
5.87 - "public class Test {" +
5.88 - " private void test () {" +
5.89 - " java.util.Map<String, Number> l = null;" +
5.90 - " l.con|tainsValue(\"\");" +
5.91 - " }" +
5.92 - "}",
5.93 - "0:113-0:132:warning:SCIT: java.util.Map.containsValue, String, Number");
5.94 - }
5.95 -
5.96 - public void testExtends1() throws Exception {
5.97 - performAnalysisTest("test/Test.java",
5.98 - "package test;" +
5.99 - "public class Test extends java.util.LinkedList<String> {" +
5.100 - " private void test () {" +
5.101 - " re|move(new Object());" +
5.102 - " }" +
5.103 - "}",
5.104 - "0:103-0:123:warning:SC: java.util.Collection.remove, Object, String");
5.105 - }
5.106 -
5.107 - public void testExtends2() throws Exception {
5.108 - performAnalysisTest("test/Test.java",
5.109 - "package test;" +
5.110 - "public class Test extends java.util.LinkedList<String> {" +
5.111 - " private void test () {" +
5.112 - " new Runnable() {" +
5.113 - " public void run() {" +
5.114 - " re|move(new Object());" +
5.115 - " }" +
5.116 - " }" +
5.117 - " }" +
5.118 - "}",
5.119 - "0:166-0:186:warning:SC: java.util.Collection.remove, Object, String");
5.120 - }
5.121 -
5.122 - @Override
5.123 - protected List<ErrorDescription> computeErrors(CompilationInfo info, TreePath tp) {
5.124 - while (tp != null) {
5.125 - if (tp.getLeaf().getKind() != Kind.METHOD_INVOCATION) {
5.126 - tp = tp.getParentPath();
5.127 - continue;
5.128 - }
5.129 - return new CollectionRemove().run(info, tp);
5.130 - }
5.131 -
5.132 - return null;
5.133 - }
5.134 -
5.135 - static {
5.136 - NbBundle.setBranding("test");
5.137 - }
5.138 -
5.139 -}