SONARJAVA-1432 Resolution of default constructor

Author: benzonico

java-checks/src/main/java/org/sonar/java/checks/AbstractClassWithoutAbstractMethodCheck.java

@@ -54,19 +54,34 @@ public void visitNode(Tree tree) {
     ClassTree classTree = (ClassTree) tree;
     Symbol.TypeSymbol typeSymbol = classTree.symbol();
     if (typeSymbol.isAbstract()) {
-      Collection<Symbol> symbols = typeSymbol.memberSymbols();
-      int abstractMethod = countAbstractMethods(symbols);
-      if (isExtendingObject(classTree) && abstractMethod == symbols.size() - 2) {
+      Collection<Symbol> members = typeSymbol.memberSymbols();
+      int nbAbstractMethod = countAbstractMethods(members);
+      // don't count this and super as members
+      int nbOfMembers = members.size() - 2;
+      if (hasDefaultConstructor(members)) {
+        //remove default constructor from members
+        nbOfMembers -=1;
+      }
+      if (isExtendingObject(classTree) && nbAbstractMethod == nbOfMembers) {
         // emtpy abstract class or only abstract method
         context.addIssue(tree, this, "Convert this \"" + typeSymbol + "\" class to an interface");
       }
-      if (symbols.size() > 2 && abstractMethod == 0 && !isPartialImplementation(classTree)) {
+      if (nbOfMembers > 0 && nbAbstractMethod == 0 && !isPartialImplementation(classTree)) {
         // Not empty abstract class with no abstract method
         context.addIssue(tree, this, "Convert this \"" + typeSymbol + "\" class to a concrete class with a private constructor");
       }
     }
   }
 
+  private boolean hasDefaultConstructor(Collection<Symbol> members) {
+    for (Symbol member : members) {
+      if ("<init>".equals(member.name()) && member.declaration() == null) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   private static boolean isExtendingObject(ClassTree tree) {
     TypeTree superClass = tree.superClass();
     return superClass == null || superClass.symbolType().is("java.lang.Object");

java-checks/src/main/java/org/sonar/java/checks/NoTestInTestClassCheck.java

@@ -95,7 +95,7 @@ private void checkClass(ClassTree classTree) {
 
   private void checkTestNGmembers(IdentifierTree className, Iterable<Symbol> members) {
     for (Symbol member : members) {
-      if (member.isMethodSymbol() && member.isPublic() && !member.isStatic()) {
+      if (member.isMethodSymbol() && member.isPublic() && !member.isStatic() && ((Symbol.MethodSymbol) member).returnType() != null) {
         return;
       }
     }

java-squid/src/main/java/org/sonar/java/resolve/FirstPass.java

@@ -20,6 +20,7 @@
 package org.sonar.java.resolve;
 
 import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import org.sonar.java.ast.api.JavaPunctuator;
 import org.sonar.java.model.declaration.ClassTreeImpl;
@@ -255,6 +256,13 @@ public void visitClass(ClassTree tree) {
     scan(tree.superClass());
     scan(tree.superInterfaces());
     scan(tree.members());
+    //Register default constructor
+    if(tree.is(Tree.Kind.CLASS) && classEnv.scope.lookup("<init>").isEmpty()) {
+      JavaSymbol.MethodJavaSymbol defaultConstructor = new JavaSymbol.MethodJavaSymbol(symbol.flags & Flags.ACCESS_FLAGS, "<init>", symbol);
+      JavaType.MethodJavaType defaultConstructorType = new JavaType.MethodJavaType(ImmutableList.<JavaType>of(), null, ImmutableList.<JavaType>of(), symbol);
+      defaultConstructor.setMethodType(defaultConstructorType);
+      classEnv.scope.enter(defaultConstructor);
+    }
     restoreEnvironment(tree);
     restoreEnvironment(tree);
   }

java-squid/src/test/files/sym/declarations/ConstructorDeclaration.java

@@ -19,4 +19,13 @@ class ParameterType {
   }
 
   ConstructorDeclaration cd = new ConstructorDeclaration(null);
+
+  ParameterType pt = new ParameterType();
+
+  abstract class Foo {}
+  class Bar extends Foo {
+    Bar(int i) {
+      super();
+    }
+  }
 }

java-squid/src/test/java/org/sonar/java/resolve/SymbolTableTest.java

@@ -404,16 +404,22 @@ public void MethodDeclaration() {
   public void ConstructorDeclaration() {
     Result result = Result.createFor("declarations/ConstructorDeclaration");
 
-    JavaSymbol.MethodJavaSymbol methodSymbol = (JavaSymbol.MethodJavaSymbol) result.symbol("<init>");
+    JavaSymbol.MethodJavaSymbol methodSymbol = (JavaSymbol.MethodJavaSymbol) result.symbol("<init>", 18);
     assertThat(methodSymbol.owner()).isSameAs(result.symbol("ConstructorDeclaration"));
     assertThat(methodSymbol.flags()).isEqualTo(0);
-    assertThat(methodSymbol.getReturnType()).isNull(); // TODO should it be result.symbol("ConstructorDeclaration")?
+    assertThat(methodSymbol.getReturnType()).isNull();
     assertThat(methodSymbol.parameterTypes()).hasSize(1);
     assertThat(methodSymbol.thrownTypes()).containsExactly(
       result.symbol("FirstExceptionType").type(),
       result.symbol("SecondExceptionType").type());
-
     assertThat(result.reference(21, 35)).isEqualTo(methodSymbol);
+    assertThat(((JavaSymbol.TypeJavaSymbol) methodSymbol.owner()).lookupSymbols("<init>")).as("Constructor with a declared constructor should not have a default one").hasSize(1);
+    //Default constructor
+    JavaSymbol defaultConstructor = result.reference(23, 26);
+    assertThat(defaultConstructor.owner).isSameAs(result.symbol("ParameterType"));
+    defaultConstructor = result.reference(28, 7);
+    assertThat(defaultConstructor.isAbstract()).isFalse();
+
   }
 
   @Test