Merge branch '__rultor'

aibolit/ast_framework/ast_node.py

@@ -64,12 +64,24 @@ def __str__(self) -> str:
         text_representation = f'node index: {self._node_index}'
         node_type = self.__getattr__('node_type')
         for attribute_name in sorted(common_attributes | attributes_by_node_type[node_type]):
-            text_representation += f'\n{attribute_name}: {self.__getattr__(attribute_name)}'
+            attribute_value = self.__getattr__(attribute_name)
+            attribute_representation = \
+                repr(attribute_value) if isinstance(attribute_value, ASTNode) else str(attribute_value)
+            text_representation += f'\n{attribute_name}: {attribute_representation}'
 
         return text_representation
 
     def __repr__(self) -> str:
         return f'<ASTNode node_type: {self.__getattr__("node_type")}, node_index: {self._node_index}>'
 
+    def __eq__(self, other: object) -> bool:
+        if not isinstance(other, ASTNode):
+            raise NotImplementedError(f'ASTNode support comparission only with themselves, \
+                                        but {type(other)} was provided.')
+        return self._graph == other._graph and self._node_index == other._node_index
+
+    def __hash__(self):
+        return hash(self._node_index)
+
     # names of methods and properties, which is not generated dynamically
     _public_fixed_interface = ['children', 'node_index', 'subtree']

aibolit/metrics/ncss/ncss.py

@@ -20,59 +20,89 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 
-from networkx import dfs_labeled_edges  # type: ignore
-
+from aibolit.ast_framework import AST, ASTNode, ASTNodeType
 from aibolit.utils.ast_builder import build_ast
-from aibolit.ast_framework import AST, ASTNodeType
 
 
 class NCSSMetric():
-    def __init__(self):
-        pass
-
-    def value(self, filename: str):
-        set_of_the_types = {ASTNodeType.ANNOTATION_DECLARATION,
-                            ASTNodeType.CLASS_DECLARATION,
-                            ASTNodeType.CONSTANT_DECLARATION,
-                            ASTNodeType.CONSTRUCTOR_DECLARATION,
-                            ASTNodeType.DECLARATION,
-                            ASTNodeType.ENUM_CONSTANT_DECLARATION,
-                            ASTNodeType.ENUM_DECLARATION,
-                            ASTNodeType.FIELD_DECLARATION,
-                            ASTNodeType.INTERFACE_DECLARATION,
-                            ASTNodeType.LOCAL_VARIABLE_DECLARATION,
-                            ASTNodeType.METHOD_DECLARATION,
-                            ASTNodeType.TYPE_DECLARATION,
-                            ASTNodeType.VARIABLE_DECLARATION,
-                            ASTNodeType.CATCH_CLAUSE,
-                            ASTNodeType.ASSERT_STATEMENT,
-                            ASTNodeType.BREAK_STATEMENT,
-                            ASTNodeType.CONTINUE_STATEMENT,
-                            ASTNodeType.DO_STATEMENT,
-                            ASTNodeType.FOR_STATEMENT,
-                            ASTNodeType.IF_STATEMENT,
-                            ASTNodeType.RETURN_STATEMENT,
-                            ASTNodeType.STATEMENT,
-                            ASTNodeType.STATEMENT_EXPRESSION,
-                            ASTNodeType.SWITCH_STATEMENT,
-                            ASTNodeType.SWITCH_STATEMENT_CASE,
-                            ASTNodeType.SYNCHRONIZED_STATEMENT,
-                            ASTNodeType.THROW_STATEMENT,
-                            ASTNodeType.TRY_STATEMENT,
-                            ASTNodeType.WHILE_STATEMENT}
-
-        if len(filename) == 0:
-            raise ValueError('Empty file for analysis')
+    '''
+    NCSS metric counts non-commenting source statements.
+    It counts keywords, declarations and statement expressions.
+    Following description was used as a reference:
+    https://pmd.github.io/latest/pmd_java_metrics_index.html#non-commenting-source-statements-ncss
+    '''
 
-        tree = AST.build_from_javalang(build_ast(filename))
+    def value(self, filename: str) -> int:
         metric = 0
-        for _, destination, edge_type in dfs_labeled_edges(tree.tree, tree.root):
-            if edge_type == 'forward':
-                node_type = tree.get_type(destination)
-                if node_type in set_of_the_types:
-                    metric += 1
-                if node_type == ASTNodeType.FOR_CONTROL:
-                    metric -= len(list(tree.children_with_type(destination, ASTNodeType.VARIABLE_DECLARATION)))
-                    metric -= len(list(tree.children_with_type(destination, ASTNodeType.LOCAL_VARIABLE_DECLARATION)))
+        ast = AST.build_from_javalang(build_ast(filename))
+        for node in ast.get_proxy_nodes(*NCSSMetric._keyword_node_types,
+                                        *NCSSMetric._declarations_node_types,
+                                        *NCSSMetric._misc_node_types):
+            metric += 1
+            if node.node_type == ASTNodeType.IF_STATEMENT and self._has_pure_else_statements(node):
+                metric += 1
+
+        for try_statement in ast.get_proxy_nodes(ASTNodeType.TRY_STATEMENT):
+            if self._has_finally_block(try_statement):
+                metric += 1
 
         return metric
+
+    def _has_pure_else_statements(self, if_statement: ASTNode) -> bool:
+        '''
+        Checks is there else branch.
+        If else branch appeared to be "else if" construction (not pure "else"),
+        returns False.
+        '''
+        assert(if_statement.node_type == ASTNodeType.IF_STATEMENT)
+        return if_statement.else_statement is not None and \
+            if_statement.else_statement.node_type != ASTNodeType.IF_STATEMENT
+
+    def _has_finally_block(self, try_statement: ASTNode) -> bool:
+        assert(try_statement.node_type == ASTNodeType.TRY_STATEMENT)
+        return try_statement.finally_block is not None
+
+    # Two keywords "else" and "finally" are not represented by any nodes
+    # and have to be extracted manually from fields
+    # of IF_STATEMENT and TRY_STATEMENT respectively
+    _keyword_node_types = {
+        ASTNodeType.ASSERT_STATEMENT,
+        ASTNodeType.BREAK_STATEMENT,
+        ASTNodeType.CATCH_CLAUSE,
+        ASTNodeType.CONTINUE_STATEMENT,
+        ASTNodeType.DO_STATEMENT,
+        ASTNodeType.FOR_STATEMENT,
+        ASTNodeType.IF_STATEMENT,
+        ASTNodeType.RETURN_STATEMENT,
+        ASTNodeType.SWITCH_STATEMENT_CASE,
+        ASTNodeType.SWITCH_STATEMENT,
+        ASTNodeType.SYNCHRONIZED_STATEMENT,
+        ASTNodeType.THROW_STATEMENT,
+        ASTNodeType.WHILE_STATEMENT,
+    }
+
+    # There are two type of declarations: type declarations (annotation, class etc.)
+    # and class parts declarations (fields, methods etc.)
+    # Package declarations are out of our interest, because it makes harder to compare
+    # nested and outer classes.
+    _declarations_node_types = {
+        ASTNodeType.ANNOTATION_DECLARATION,
+        ASTNodeType.CLASS_DECLARATION,
+        ASTNodeType.CONSTANT_DECLARATION,
+        ASTNodeType.CONSTRUCTOR_DECLARATION,
+        ASTNodeType.ENUM_CONSTANT_DECLARATION,
+        ASTNodeType.ENUM_DECLARATION,
+        ASTNodeType.FIELD_DECLARATION,
+        ASTNodeType.INTERFACE_DECLARATION,
+        ASTNodeType.METHOD_DECLARATION,
+        ASTNodeType.TYPE_DECLARATION,
+    }
+
+    _misc_node_types = {
+        # Represent declaration of all local variables
+        # *excluding* declarations inside "for" loop control.
+        # Those declarations are represented by "VARIABLE_DECLARATION" nodes.
+        ASTNodeType.LOCAL_VARIABLE_DECLARATION,
+        # Statement expressions also includes assignments
+        ASTNodeType.STATEMENT_EXPRESSION,
+    }

test/metrics/ncss/BasicExample.java

@@ -1,25 +1,27 @@
-import java.util.Collections;       // +0
-import java.io.IOException;         // +0
+// Total NCSS = 12
 
-class Foo {                         // +1, total Ncss = 12
-
-  public void bigMethod()           // +1
-      throws IOException {     
-    int x = 0, y = 2;               // +1
-    boolean a = false, b = true;    // +1
-
-    if (a || b) {                   // +1
-      try {                         // +1
-        do {                        // +1
-          x += 2;                   // +1
+import java.util.Collections;
+import java.io.IOException;
+
+class Foo {                                   // +1
+
+  public void bigMethod()                     // +1
+      throws IOException {
+    int x = 0, y = 2;                         // +1
+    boolean a = false, b = true;              // +1
+
+    if (a || b) {                             // +1
+      try {
+        do {                                  // +1
+          x += 2;                             // +1
         } while (x < 12);
-          
-        System.exit(0);             // +1
-      } catch (IOException ioe) {   // +1
+
+        System.exit(0);                       // +1
+      } catch (IOException ioe) {             // +1
         throw new PatheticFailException(ioe); // +1
       }
-    } else {
-      assert false;                 // +1
+    } else {                                  // +1
+      assert false;                           // +1
     }
-  }     
+  }
 }

test/metrics/ncss/ChainedIfElse.java

@@ -0,0 +1,17 @@
+// Total NCSS = 11
+
+class ChainedIfElse {                           // +1
+    public int chainedIfElseMethod(int x) {     // +1
+        if(x > 10) {                            // +1
+            return 100;                         // +1
+        } else if(x > 5) {                      // +1
+            return 5;                           // +1
+        } else if (x > 0) {                     // +1
+            return 1;                           // +1
+        } else if (x == 0) {                    // +1
+            return 0;                           // +1
+        }
+
+        return -1;                              // +1
+    }
+}

test/metrics/ncss/ChainedIfElseWithTrailingElse.java

@@ -0,0 +1,18 @@
+// Total NCSS = 12
+
+class ChainedIfElse {                           // +1
+    public int chainedIfElseMethod(int x) {     // +1
+        if(x > 10) {                            // +1
+            return 100;                         // +1
+        } else if(x > 5) {                      // +1
+            return 5;                           // +1
+        } else if (x > 0) {                     // +1
+            return 1;                           // +1
+        } else if (x == 0) {                    // +1
+            return 0;                           // +1
+        }
+        else {                                  // +1
+            return -1;                          // +1
+        }
+    }
+}

test/metrics/ncss/Empty.java

@@ -1 +1 @@
-
+// Total NCSS = 0

test/metrics/ncss/FinallyBlock.java

@@ -0,0 +1,16 @@
+// Total NCSS = 6
+
+class FinallyBlock {                                        // +1
+    private int x = 0;                                      // +1
+
+    public void tryIncrement() {                            // +1
+        try {
+            // Increment will never raise exception,
+            // but it can be replaced with something
+            // more dangerous.
+            x += 1;                                         // +1
+        } finally {                                         // +1
+            x = 0;                                          // +1
+        }
+    }
+}

test/metrics/ncss/Simple.java

@@ -1,9 +1,11 @@
-import java.util.Collections;       
-import java.io.IOException;         
+// Total NCSS = 2
 
-class Foo {                       
-
-  public void bigMethod() {
+import java.util.Collections;
+import java.io.IOException;
+
+class Foo {                 // +1
+
+  public void bigMethod() { // +1
   }
 }
 

test/metrics/ncss/SimpleExample.java

@@ -1,29 +1,31 @@
-import java.util.Collections;       // +0
-import java.io.IOException;         // +0
+// Total NCSS = 17
 
-class Foo {                         // +1, total Ncss = 16
-
-  public void bigMethod()           // +1
-      throws IOException {     
-    int x, y = 2;               // +1
-    boolean a, b;               // +1
-    a = false;                  // +1
-    b = true;                   // +1 
-    x = 0;                      // +1
-    if (a || b) {                   // +1
-      try {                         // +1
-        do {                        // +1
-          x += 2;                   // +1
+import java.util.Collections;
+import java.io.IOException;
+
+class Foo {                                   // +1
+
+  public void bigMethod()                     // +1
+      throws IOException {
+    int x, y = 2;                             // +1
+    boolean a, b;                             // +1
+    a = false;                                // +1
+    b = true;                                 // +1
+    x = 0;                                    // +1
+    if (a || b) {                             // +1
+      try {
+        do {                                  // +1
+          x += 2;                             // +1
         } while (x < 12);
-          
-        System.exit(0);             // +1
-      } catch (IOException ioe) {   // +1
+
+        System.exit(0);                       // +1
+      } catch (IOException ioe) {             // +1
         throw new PatheticFailException(ioe); // +1
-      } finally {                   // +1
-        System.out.println("Finally Block");
+      } finally {                             // +1
+        System.out.println("Finally Block");  // +1
       }
-    } else {
-      assert false;                 // +1
+    } else {                                  // +1
+      assert false;                           // +1
     }
-  }     
+  }
 }

test/metrics/ncss/SimpleExample2.java

@@ -1,29 +1,31 @@
-import java.util.Collections;       // +0
-import java.io.IOException;         // +0
+// Total NCSS = 18
 
-class Foo {                         // +1, total Ncss = 16
-
-  public void bigMethod()           // +1
-      throws IOException {     
-    int x, y = 2;               // +1
-    boolean a, b;               // +1
-    a = false;                  // +1
-    b = true;                   // +1 
-    x = 0;                      // +1
-    if (a || b) {                   // +1
-      try {                         // +1
-        for(int i = 0, j = 10; i < j; i++){      // +1
-            x += 2;                              // +1
+import java.util.Collections;
+import java.io.IOException;
+
+class Foo {                                   // +1
+  public void bigMethod()                     // +1
+      throws IOException {
+    int x, y = 2;                             // +1
+    boolean a, b;                             // +1
+    a = false;                                // +1
+    b = true;                                 // +1
+    x = 0;                                    // +1
+    if (a || b) {                             // +1
+      try {
+        int i, j;                             // +1
+        for(i = 0, j = 10; i < j; i++) {      // +1
+          x += 2;                             // +1
         }
-          
-        System.exit(0);             // +1
-      } catch (IOException ioe) {   // +1
+
+        System.exit(0);                       // +1
+      } catch (IOException ioe) {             // +1
         throw new PatheticFailException(ioe); // +1
-      } finally {                   // +1
-        System.out.println("Finally Block");
+      } finally {                             // +1
+        System.out.println("Finally Block");  // +1
       }
-    } else {
-      assert false;                 // +1
+    } else {                                  // +1
+      assert false;                           // +1
     }
-  }     
+  }
 }