Bug 376590 - Private fields with @Inject are ignored by unused

field validation

4 files changed, 155 insertions, 31 deletions

diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AbstractComparableTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AbstractComparableTest.java
index a37199940f..22685c160f 100644
--- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AbstractComparableTest.java
+++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AbstractComparableTest.java
@@ -7,6 +7,8 @@
  *
  * Contributors:
  *     IBM Corporation - initial API and implementation
+ *     Stephan Herrmann - Contribution for
+ *								bug 376590 - Private fields with @Inject are ignored by unused field validation
  *******************************************************************************/
 package org.eclipse.jdt.core.tests.compiler.regression;
 
@@ -15,6 +17,32 @@ import junit.framework.Test;
 import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
 
 public class AbstractComparableTest extends AbstractRegressionTest {
+
+	protected static final String GOOGLE_INJECT_NAME = "com/google/inject/Inject.java";
+	protected static final String GOOGLE_INJECT_CONTENT = 
+		"package com.google.inject;\n" + 
+		"import static java.lang.annotation.ElementType.*;\n" + 
+		"import java.lang.annotation.Retention;\n" + 
+		"import static java.lang.annotation.RetentionPolicy.RUNTIME;\n" + 
+		"import java.lang.annotation.Target;\n" +
+		"@Target({ METHOD, CONSTRUCTOR, FIELD })\n" + 
+		"@Retention(RUNTIME)\n" + 
+		"public @interface Inject {\n" + 
+		"\n" + 
+		"  boolean optional() default false;\n" + 
+		"}";
+
+	protected static final String JAVAX_INJECT_NAME = "javax/inject/Inject.java";
+	protected static final String JAVAX_INJECT_CONTENT = 
+		"package javax.inject;\n" + 
+		"import static java.lang.annotation.ElementType.*;\n" + 
+		"import java.lang.annotation.Retention;\n" + 
+		"import static java.lang.annotation.RetentionPolicy.RUNTIME;\n" + 
+		"import java.lang.annotation.Target;\n" +
+		"@Target({ METHOD, CONSTRUCTOR, FIELD })\n" + 
+		"@Retention(RUNTIME)\n" + 
+		"public @interface Inject {}\n";
+
 	public static Test buildComparableTestSuite(Class evaluationTestClass) {
 		Test suite = buildMinimalComplianceTestSuite(evaluationTestClass, F_1_5);
 		TESTS_COUNTERS.put(evaluationTestClass.getName(), new Integer(suite.countTestCases()));
diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AnnotationTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AnnotationTest.java
index 9608c0539d..0c3182aafb 100644
--- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AnnotationTest.java
+++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AnnotationTest.java
@@ -14,6 +14,7 @@
  *								bug 384663 - Package Based Annotation Compilation Error in JDT 3.8/4.2 (works in 3.7.2) 
  *								bug 386356 - Type mismatch error with annotations and generics
  *								bug 331649 - [compiler][null] consider null annotations for fields
+ *								bug 376590 - Private fields with @Inject are ignored by unused field validation
  *******************************************************************************/
 package org.eclipse.jdt.core.tests.compiler.regression;
 
@@ -10520,6 +10521,119 @@ public void testBug365437f() {
 		"----------\n",
 		JavacTestOptions.Excuse.EclipseWarningConfiguredAsError);
 }
+
+// https://bugs.eclipse.org/376590 - Private fields with @Inject are ignored by unused field validation
+// using com.google.inject.Inject
+public void testBug376590a() {
+	Map customOptions = getCompilerOptions();
+	customOptions.put(CompilerOptions.OPTION_ReportUnusedPrivateMember, CompilerOptions.ERROR);
+	customOptions.put(CompilerOptions.OPTION_ReportUnusedPrivateMember, CompilerOptions.ERROR);
+	this.runNegativeTest(
+		true,
+		new String[] {
+			GOOGLE_INJECT_NAME,
+			GOOGLE_INJECT_CONTENT,
+			"Example.java",
+			"import com.google.inject.Inject;\n" +
+			"class Example {\n" +
+			"  private @Inject Object o;\n" +
+			"  private @Inject Example() {}\n" + // no warning on constructor
+			"  public Example(Object o) { this.o = o; }\n" +
+			"  private @Inject void setO(Object o) { this.o = o;}\n" + // no warning on method
+			"}\n"
+		},
+		null, customOptions,
+		"----------\n" + 
+		"1. ERROR in Example.java (at line 3)\n" + 
+		"	private @Inject Object o;\n" + 
+		"	                       ^\n" + 
+		"The value of the field Example.o is not used\n" + 
+		"----------\n",
+		JavacTestOptions.Excuse.EclipseWarningConfiguredAsError);	
+}
+// https://bugs.eclipse.org/376590 - Private fields with @Inject are ignored by unused field validation
+// using javax.inject.Inject - slight variation
+public void testBug376590b() {
+	Map customOptions = getCompilerOptions();
+	customOptions.put(CompilerOptions.OPTION_ReportUnusedPrivateMember, CompilerOptions.ERROR);
+	customOptions.put(CompilerOptions.OPTION_ReportUnusedPrivateMember, CompilerOptions.ERROR);
+	this.runNegativeTest(
+		true,
+		new String[] {
+			JAVAX_INJECT_NAME,
+			JAVAX_INJECT_CONTENT,
+			"Example.java",
+			"class Example {\n" +
+			"  private @javax.inject.Inject Object o;\n" +
+			"  private Example() {} // also warn here: no @Inject\n" +
+			"  public Example(Object o) { this.o = o; }\n" +
+			"  private @javax.inject.Inject void setO(Object o) { this.o = o;}\n" +
+			"}\n"
+		},
+		null, customOptions,
+		"----------\n" + 
+		"1. ERROR in Example.java (at line 2)\n" + 
+		"	private @javax.inject.Inject Object o;\n" + 
+		"	                                    ^\n" + 
+		"The value of the field Example.o is not used\n" + 
+		"----------\n" + 
+		"2. ERROR in Example.java (at line 3)\n" + 
+		"	private Example() {} // also warn here: no @Inject\n" + 
+		"	        ^^^^^^^^^\n" + 
+		"The constructor Example() is never used locally\n" + 
+		"----------\n",
+		JavacTestOptions.Excuse.EclipseWarningConfiguredAsError);	
+}
+// https://bugs.eclipse.org/376590 - Private fields with @Inject are ignored by unused field validation
+// using javax.inject.Inject, combined with standard as well as custom annotations
+public void testBug376590c() {
+	Map customOptions = getCompilerOptions();
+	customOptions.put(CompilerOptions.OPTION_ReportUnusedPrivateMember, CompilerOptions.ERROR);
+	customOptions.put(CompilerOptions.OPTION_ReportUnusedPrivateMember, CompilerOptions.ERROR);
+	customOptions.put(CompilerOptions.OPTION_AnnotationBasedNullAnalysis, CompilerOptions.ENABLED);
+	customOptions.put(CompilerOptions.OPTION_NonNullAnnotationName, "p.NonNull");
+	this.runNegativeTest(
+		true,
+		new String[] {
+			JAVAX_INJECT_NAME,
+			JAVAX_INJECT_CONTENT,
+			"Example.java",
+			"import javax.inject.Inject;\n" +
+			"class Example {\n" +
+			"  private @Inject @p.NonNull Object o; // do warn, annotations don't signal a read\n" +
+			"  private @Deprecated @Inject String old; // do warn, annotations don't signal a read\n" +
+			"  private @Inject @p.Annot Object o2;\n" + // don't warn, custom annotation could imply a read access
+			"}\n",
+			"p/NonNull.java",
+			"package p;\n" +
+			"import static java.lang.annotation.ElementType.*;\n" +
+			"import java.lang.annotation.*;\n" +
+			"@Target({TYPE, METHOD,PARAMETER,LOCAL_VARIABLE,FIELD})\n" +
+			"public @interface NonNull {\n" +
+			"}",
+			"p/Annot.java",
+			"package p;\n" +
+			"import static java.lang.annotation.ElementType.*;\n" +
+			"import java.lang.annotation.*;\n" +
+			"@Target({TYPE, METHOD,PARAMETER,LOCAL_VARIABLE, CONSTRUCTOR, FIELD})\n" +
+			"public @interface Annot {\n" +
+			"}"
+		},
+		null, customOptions,
+		"----------\n" + 
+		"1. ERROR in Example.java (at line 3)\n" + 
+		"	private @Inject @p.NonNull Object o; // do warn, annotations don't signal a read\n" + 
+		"	                                  ^\n" + 
+		"The value of the field Example.o is not used\n" + 
+		"----------\n" + 
+		"2. ERROR in Example.java (at line 4)\n" + 
+		"	private @Deprecated @Inject String old; // do warn, annotations don't signal a read\n" + 
+		"	                                   ^^^\n" + 
+		"The value of the field Example.old is not used\n" + 
+		"----------\n",
+		JavacTestOptions.Excuse.EclipseWarningConfiguredAsError);	
+}
+
 public void testBug376429a() {
 	this.runNegativeTest(
 			new String[] {
diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullAnnotationTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullAnnotationTest.java
index 7d77c86c89..b8b13eca54 100644
--- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullAnnotationTest.java
+++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullAnnotationTest.java
@@ -46,31 +46,6 @@ private static final String CUSTOM_NULLABLE_CONTENT = "package org.foo;\n" +
 	"public @interface Nullable {\n" +
 	"}\n";
 
-private static final String GOOGLE_INJECT_NAME = "com/google/inject/Inject.java";
-private static final String GOOGLE_INJECT_CONTENT = 
-	"package com.google.inject;\n" + 
-	"import static java.lang.annotation.ElementType.*;\n" + 
-	"import java.lang.annotation.Retention;\n" + 
-	"import static java.lang.annotation.RetentionPolicy.RUNTIME;\n" + 
-	"import java.lang.annotation.Target;\n" +
-	"@Target({ METHOD, CONSTRUCTOR, FIELD })\n" + 
-	"@Retention(RUNTIME)\n" + 
-	"public @interface Inject {\n" + 
-	"\n" + 
-	"  boolean optional() default false;\n" + 
-	"}";
-
-private static final String JAVAX_INJECT_NAME = "javax/inject/Inject.java";
-private static final String JAVAX_INJECT_CONTENT = 
-	"package javax.inject;\n" + 
-	"import static java.lang.annotation.ElementType.*;\n" + 
-	"import java.lang.annotation.Retention;\n" + 
-	"import static java.lang.annotation.RetentionPolicy.RUNTIME;\n" + 
-	"import java.lang.annotation.Target;\n" +
-	"@Target({ METHOD, CONSTRUCTOR, FIELD })\n" + 
-	"@Retention(RUNTIME)\n" + 
-	"public @interface Inject {}\n";
-
 public NullAnnotationTest(String name) {
 	super(name);
 }
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java
index 34cf3a368d..b4a62a557d 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java
@@ -24,6 +24,7 @@
  *								bug 393719 - [compiler] inconsistent warnings on iteration variables
  *								bug 331649 - [compiler][null] consider null annotations for fields
  *								bug 382789 - [compiler][null] warn when syntactically-nonnull expression is compared against null
+ *								bug 376590 - Private fields with @Inject are ignored by unused field validation
  *******************************************************************************/
 package org.eclipse.jdt.internal.compiler.problem;
 
@@ -7831,7 +7832,7 @@ public void unusedPrivateConstructor(ConstructorDeclaration constructorDecl) {
 	int severity = computeSeverity(IProblem.UnusedPrivateConstructor);
 	if (severity == ProblemSeverities.Ignore) return;
 	
-	if (excludeDueToAnnotation(constructorDecl.annotations)) return;
+	if (excludeDueToAnnotation(constructorDecl.annotations, IProblem.UnusedPrivateConstructor)) return;
 	
 	MethodBinding constructor = constructorDecl.binding;
 	this.handle(
@@ -7878,7 +7879,7 @@ public void unusedPrivateField(FieldDeclaration fieldDecl) {
 			}
 		}
 	}
-	if (excludeDueToAnnotation(fieldDecl.annotations)) return;
+	if (excludeDueToAnnotation(fieldDecl.annotations, IProblem.UnusedPrivateField)) return;
 	this.handle(
 			IProblem.UnusedPrivateField,
 		new String[] {
@@ -7932,7 +7933,7 @@ public void unusedPrivateMethod(AbstractMethodDeclaration methodDecl) {
 			&& CharOperation.equals(method.selector, TypeConstants.WRITEREPLACE)) {
 		return;
 	}
-	if (excludeDueToAnnotation(methodDecl.annotations)) return;
+	if (excludeDueToAnnotation(methodDecl.annotations, IProblem.UnusedPrivateMethod)) return;
 	
 	this.handle(
 			IProblem.UnusedPrivateMethod,
@@ -7954,9 +7955,10 @@ public void unusedPrivateMethod(AbstractMethodDeclaration methodDecl) {
 /**
  * Returns true if a private member should not be warned as unused if
  * annotated with a non-standard annotation.
- * https://bugs.eclipse.org/bugs/show_bug.cgi?id=365437
+ * https://bugs.eclipse.org/365437
+ * https://bugs.eclipse.org/376590
  */
-private boolean excludeDueToAnnotation(Annotation[] annotations) {
+private boolean excludeDueToAnnotation(Annotation[] annotations, int problemId) {
 	int annotationsLen = 0;
 	if (annotations != null) {
 		annotationsLen = annotations.length;
@@ -7975,6 +7977,11 @@ private boolean excludeDueToAnnotation(Annotation[] annotations) {
 				case TypeIds.T_ConfiguredAnnotationNullable:
 				case TypeIds.T_ConfiguredAnnotationNonNullByDefault:
 					break;
+				case TypeIds.T_JavaxInjectInject:
+				case TypeIds.T_ComGoogleInjectInject:
+					if (problemId != IProblem.UnusedPrivateField)
+						return true; // @Inject on method/ctor does constitute a relevant use, just on fields it doesn't
+					break;
 				default:
 					// non-standard annotation found, don't warn
 					return true;
@@ -7986,7 +7993,7 @@ private boolean excludeDueToAnnotation(Annotation[] annotations) {
 public void unusedPrivateType(TypeDeclaration typeDecl) {
 	int severity = computeSeverity(IProblem.UnusedPrivateType);
 	if (severity == ProblemSeverities.Ignore) return;
-	if (excludeDueToAnnotation(typeDecl.annotations)) return;
+	if (excludeDueToAnnotation(typeDecl.annotations, IProblem.UnusedPrivateType)) return;
 	ReferenceBinding type = typeDecl.binding;
 	this.handle(
 			IProblem.UnusedPrivateType,