From 080fe6a3983cfc15f47733ae2378d889a3bf0086 Mon Sep 17 00:00:00 2001
From: Bas Leijdekkers <basleijdekkers@gmail.com>
Date: Thu, 20 Mar 2008 10:42:07 +0300
Subject: [PATCH] IDEADEV-25371 and some cleanup

---
 .../com/siyeh/InspectionGadgetsBundle.properties   |   2 +-
 .../ig/resources/ChannelResourceInspection.java    | 135 ++++++++++++++-------
 .../siyeh/ig/resources/JNDIResourceInspection.java |  34 ++----
 .../com/siyeh/ig/threading/SafeLockInspection.java |   3 +-
 .../inspectionDescriptions/ChannelResource.html    |  22 ++--
 .../siyeh/igtest/threading/SafeLockInspection.java |   2 +-
 6 files changed, 111 insertions(+), 87 deletions(-)
 rewrite plugins/InspectionGadgets/src/inspectionDescriptions/ChannelResource.html (67%)

diff --git a/plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties
index a6835523fa..e9edf1fc1b 100644
--- a/plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties
+++ b/plugins/InspectionGadgets/src/com/siyeh/InspectionGadgetsBundle.properties
@@ -383,7 +383,7 @@ anonymous.class.parameter.hides.containing.method.variable.problem.descriptor=An
 anonymous.class.field.hides.containing.method.variable.problem.descriptor=Anonymous class field <code>#ref</code> hides variable in containing method
 anonymous.class.variable.hides.containing.method.variable.problem.descriptor=Anonymous class local variable <code>#ref</code> hides variable in containing method
 channel.opened.not.closed.display.name=Channel opened but not safely closed
-channel.opened.not.closed.problem.descriptor=''{0}'' should be opened in a try block, and closed in a finally block #loc
+channel.opened.not.closed.problem.descriptor=''{0}'' should be opened in front of a try block and closed in the corresponding finally block #loc
 drivermanager.call.display.name=Use of DriverManager to get JDBC connection
 drivermanager.call.problem.descriptor=Call to <code>DriverManager.#ref()</code> #loc
 hibernate.resource.opened.not.closed.display.name=Hibernate resource opened but not safely closed
diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/resources/ChannelResourceInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/resources/ChannelResourceInspection.java
index 08c84f413b..cc16c3b072 100644
--- a/plugins/InspectionGadgets/src/com/siyeh/ig/resources/ChannelResourceInspection.java
+++ b/plugins/InspectionGadgets/src/com/siyeh/ig/resources/ChannelResourceInspection.java
@@ -24,7 +24,7 @@ import com.siyeh.ig.BaseInspectionVisitor;
 import com.siyeh.ig.psiutils.TypeUtils;
 import org.jetbrains.annotations.NotNull;
 
-public class ChannelResourceInspection extends BaseInspection {
+public class ChannelResourceInspection extends BaseInspection{
 
     @NotNull
     public String getID(){
@@ -56,43 +56,64 @@ public class ChannelResourceInspection extends BaseInspection {
         @Override public void visitMethodCallExpression(
                 @NotNull PsiMethodCallExpression expression){
             super.visitMethodCallExpression(expression);
-            if(!isChannelFactoryMethod(expression)) {
+            if(!isChannelFactoryMethod(expression)){
                 return;
             }
             final PsiElement parent = expression.getParent();
-            if(!(parent instanceof PsiAssignmentExpression)) {
+            final PsiVariable boundVariable;
+            if (parent instanceof PsiAssignmentExpression) {
+                final PsiAssignmentExpression assignment =
+                        (PsiAssignmentExpression) parent;
+                final PsiExpression lhs = assignment.getLExpression();
+                if (!(lhs instanceof PsiReferenceExpression)) {
+                    return;
+                }
+                final PsiReferenceExpression referenceExpression =
+                        (PsiReferenceExpression) lhs;
+                final PsiElement referent = referenceExpression.resolve();
+                if (referent == null || !(referent instanceof PsiVariable)) {
+                    return;
+                }
+                boundVariable = (PsiVariable) referent;
+            } else if (parent instanceof PsiVariable) {
+                boundVariable = (PsiVariable) parent;
+            } else {
                 registerError(expression, expression);
                 return;
             }
-            final PsiAssignmentExpression assignment =
-                    (PsiAssignmentExpression) parent;
-            final PsiExpression lhs = assignment.getLExpression();
-            if(!(lhs instanceof PsiReferenceExpression)) {
+            final PsiStatement statement =
+                    PsiTreeUtil.getParentOfType(expression, PsiStatement.class);
+            if (statement == null) {
                 return;
             }
-            final PsiElement referent =
-                    ((PsiReference) lhs).resolve();
-            if(referent == null || !(referent instanceof PsiVariable)) {
+            PsiStatement nextStatement =
+                    PsiTreeUtil.getNextSiblingOfType(statement,
+                            PsiStatement.class);
+            if (!(nextStatement instanceof PsiTryStatement)) {
+                registerError(expression, expression);
                 return;
             }
-            final PsiVariable boundVariable = (PsiVariable) referent;
-
-            PsiElement currentContext = expression;
-            while(true){
-                final PsiTryStatement tryStatement =
-                        PsiTreeUtil.getParentOfType(currentContext,
-                                PsiTryStatement.class);
-                if(tryStatement == null) {
-                    registerError(expression, expression);
-                    return;
-                }
-                if(resourceIsOpenedInTryAndClosedInFinally(tryStatement,
-                        expression,
-                        boundVariable)) {
-                    return;
-                }
-                currentContext = tryStatement;
+            //while (!(nextStatement instanceof PsiTryStatement)) {
+            //if (!(nextStatement instanceof PsiDeclarationStatement)) {
+            //    registerError(expression, expression);
+            //    return;
+            //}
+            //final VariableReferenceVisitor visitor =
+            //        new VariableReferenceVisitor(boundVariable);
+            //nextStatement.accept(visitor);
+            //if (visitor.containsReference()) {
+            //    registerError(expression, expression);
+            //    return;
+            //}
+            //nextStatement =
+            //        PsiTreeUtil.getNextSiblingOfType(nextStatement,
+            //                PsiStatement.class);
+            //}
+            PsiTryStatement tryStatement = (PsiTryStatement) nextStatement;
+            if (resourceIsClosedInFinally(tryStatement, boundVariable)) {
+                return;
             }
+            registerError(expression, expression);
         }
 
         private static boolean isChannelFactoryMethod(
@@ -117,9 +138,8 @@ public class ChannelResourceInspection extends BaseInspection {
 		            "java.io.RandomAccessFile");
         }
 
-        private static boolean resourceIsOpenedInTryAndClosedInFinally(
-                PsiTryStatement tryStatement, PsiExpression lhs,
-                PsiVariable boundVariable){
+        private static boolean resourceIsClosedInFinally(
+                PsiTryStatement tryStatement, PsiVariable boundVariable){
             final PsiCodeBlock finallyBlock = tryStatement.getFinallyBlock();
             if(finallyBlock == null){
                 return false;
@@ -128,27 +148,51 @@ public class ChannelResourceInspection extends BaseInspection {
             if(tryBlock == null){
                 return false;
             }
-            if(!PsiTreeUtil.isAncestor(tryBlock, lhs, true)){
-                return false;
+            final CloseVisitor visitor = new CloseVisitor(boundVariable);
+            finallyBlock.accept(visitor);
+            return visitor.containsStreamClose();
+        }
+    }
+
+    /*
+    private static class VariableReferenceVisitor
+            extends JavaRecursiveElementVisitor{
+
+        private boolean containsReference = false;
+        private PsiVariable variable;
+
+        VariableReferenceVisitor(@NotNull PsiVariable variable) {
+            this.variable = variable;
+        }
+
+        @Override
+        public void visitReferenceExpression(
+                PsiReferenceExpression expression) {
+            if (containsReference) {
+                return;
+            }
+            final PsiExpression qualifier = expression.getQualifierExpression();
+            if (qualifier != null) {
+                return;
+            }
+            final PsiElement target = expression.resolve();
+            if (variable.equals(target)) {
+                containsReference = true;
             }
-            return containsResourceClose(finallyBlock, boundVariable);
         }
 
-        private static boolean containsResourceClose(PsiCodeBlock finallyBlock,
-                                                     PsiVariable boundVariable){
-            final CloseVisitor visitor =
-                    new CloseVisitor(boundVariable);
-            finallyBlock.accept(visitor);
-            return visitor.containsStreamClose();
+        public boolean containsReference() {
+            return containsReference;
         }
     }
+    */
 
     private static class CloseVisitor extends JavaRecursiveElementVisitor{
+
         private boolean containsClose = false;
         private PsiVariable objectToClose;
 
         private CloseVisitor(PsiVariable objectToClose){
-            super();
             this.objectToClose = objectToClose;
         }
 
@@ -175,13 +219,10 @@ public class ChannelResourceInspection extends BaseInspection {
             if(!(qualifier instanceof PsiReferenceExpression)){
                 return;
             }
-            final PsiElement referent =
-                    ((PsiReference) qualifier).resolve();
-            if(referent == null)
-            {
-                return;
-            }
-            if(referent.equals(objectToClose)){
+            final PsiReferenceExpression referenceExpression =
+                    (PsiReferenceExpression) qualifier;
+            final PsiElement referent = referenceExpression.resolve();
+            if(objectToClose.equals(referent)){
                 containsClose = true;
             }
         }
diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/resources/JNDIResourceInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/resources/JNDIResourceInspection.java
index 08570996d2..1dde99f973 100644
--- a/plugins/InspectionGadgets/src/com/siyeh/ig/resources/JNDIResourceInspection.java
+++ b/plugins/InspectionGadgets/src/com/siyeh/ig/resources/JNDIResourceInspection.java
@@ -40,8 +40,12 @@ public class JNDIResourceInspection extends BaseInspection {
 
     @NotNull
     public String buildErrorString(Object... infos){
+        final PsiExpression expression = (PsiExpression) infos[0];
+        final PsiType type = expression.getType();
+        assert type != null;
+        final String text = type.getPresentableText();
         return InspectionGadgetsBundle.message(
-                "resource.opened.not.closed.problem.descriptor", infos[0]);
+                "resource.opened.not.closed.problem.descriptor", text);
     }
 
     public BaseInspectionVisitor buildVisitor(){
@@ -62,12 +66,7 @@ public class JNDIResourceInspection extends BaseInspection {
             }
             final PsiElement parent = expression.getParent();
             if(!(parent instanceof PsiAssignmentExpression)){
-                final PsiType type = expression.getType();
-                if (type == null) {
-                    return;
-                }
-                final String text = type.getPresentableText();
-                registerError(expression, text);
+                registerError(expression, expression);
                 return;
             }
             final PsiAssignmentExpression assignment =
@@ -89,12 +88,7 @@ public class JNDIResourceInspection extends BaseInspection {
                         PsiTreeUtil.getParentOfType(currentContext,
                                 PsiTryStatement.class);
                 if(tryStatement == null){
-                    final PsiType type = expression.getType();
-                    if (type == null) {
-                        return;
-                    }
-                    final String text = type.getPresentableText();
-                    registerError(expression, text);
+                    registerError(expression, expression);
                     return;
                 }
                 if(resourceIsOpenedInTryAndClosedInFinally(tryStatement,
@@ -118,12 +112,7 @@ public class JNDIResourceInspection extends BaseInspection {
             }
             final PsiElement parent = expression.getParent();
             if(!(parent instanceof PsiAssignmentExpression)){
-                final PsiType type = expression.getType();
-                if (type == null) {
-                    return;
-                }
-                final String text = type.getPresentableText();
-                registerError(expression, text);
+                registerError(expression, expression);
                 return;
             }
             final PsiAssignmentExpression assignment =
@@ -145,12 +134,7 @@ public class JNDIResourceInspection extends BaseInspection {
                         PsiTreeUtil.getParentOfType(currentContext,
                                 PsiTryStatement.class);
                 if(tryStatement == null){
-                    final PsiType type = expression.getType();
-                    if (type == null) {
-                        return;
-                    }
-                    final String text = type.getPresentableText();
-                    registerError(expression, text);
+                    registerError(expression, expression);
                     return;
                 }
                 if(resourceIsOpenedInTryAndClosedInFinally(tryStatement,
diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/threading/SafeLockInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/threading/SafeLockInspection.java
index 97e0e8eb38..dc0b90295f 100644
--- a/plugins/InspectionGadgets/src/com/siyeh/ig/threading/SafeLockInspection.java
+++ b/plugins/InspectionGadgets/src/com/siyeh/ig/threading/SafeLockInspection.java
@@ -84,8 +84,7 @@ public class SafeLockInspection extends BaseInspection {
                 return;
             }
             PsiTryStatement tryStatement = (PsiTryStatement) nextStatement;
-            if (lockIsUnlockedInFinally(tryStatement,
-                    boundVariable)) {
+            if (lockIsUnlockedInFinally(tryStatement, boundVariable)) {
                 return;
             }
             registerError(expression, referenceExpression);
diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/ChannelResource.html b/plugins/InspectionGadgets/src/inspectionDescriptions/ChannelResource.html
dissimilarity index 67%
index 5afc1cd1ef..e014b2a755 100644
--- a/plugins/InspectionGadgets/src/inspectionDescriptions/ChannelResource.html
+++ b/plugins/InspectionGadgets/src/inspectionDescriptions/ChannelResource.html
@@ -1,11 +1,11 @@
-
-<html>
-<body><table> <tr> <td valign="top" height="150">
-<font face="verdana" size="-1">
-This inspection reports any Channel which is not opened in a <b><font color="#000080">try</font></b>
-block and closed in the corresponding <b><font color="#000080">finally</font></b> block.  Such resources may
-be inadvertantly leaked if an exception is thrown before the resouce is closed.  Channel resources reported
-by this inspection include any instances created by calling <b><font color="#000080">getChannel()</font></b>
-on a file or socket resource.
-</font></td> </tr> <tr> <td height="20"> <font face="verdana" size="-2">Powered by InspectionGadgets </font> </td> </tr> </table> </body>
-</html>
\ No newline at end of file
+<html>
+<body><table> <tr> <td valign="top" height="150">
+<font face="verdana" size="-1">
+This inspection reports any <b><font color="#000080">Channel</font></b> which is not opened in
+front of a <b><font color="#000080">try</font></b> block and closed in the corresponding
+<b><font color="#000080">finally</font></b> block.  Such resources may be inadvertantly leaked
+if an exception is thrown before the resouce is closed.  Channel resources reported
+by this inspection include any instances created by calling
+<b><font color="#000080">getChannel()</font></b> on a file or socket resource.
+</font></td> </tr> <tr> <td height="20"> <font face="verdana" size="-2">Powered by InspectionGadgets </font> </td> </tr> </table> </body>
+</html>
\ No newline at end of file
diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/threading/SafeLockInspection.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/threading/SafeLockInspection.java
index af61dc7551..1170c2fd6f 100644
--- a/plugins/InspectionGadgets/test/com/siyeh/igtest/threading/SafeLockInspection.java
+++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/threading/SafeLockInspection.java
@@ -21,8 +21,8 @@ public class SafeLockInspection{
     public void test3()
     {
         final Lock lock = new ReentrantLock();
+        lock.lock();
         try{
-            lock.lock();
         } finally{
             lock.unlock();
         }
-- 
2.11.4.GIT