Bug 571507 - [16] Report Warning for usage of Value Based classes for
Synchronized statements

Change-Id: I8cc34af0e00ced1441248a6403210332965463b2
Signed-off-by: Kalyan Prasad Tatavarthi <kalyan_prasad@in.ibm.com>
diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CompilerInvocationTests.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CompilerInvocationTests.java
index 110284c..0c013f9 100644
--- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CompilerInvocationTests.java
+++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CompilerInvocationTests.java
@@ -1288,6 +1288,7 @@
 	    expectedProblemAttributes.put("SealedLocalDirectSuperTypeSealed", new ProblemAttributes(CategorizedProblem.CAT_PREVIEW_RELATED));
 	    expectedProblemAttributes.put("SealedAnonymousClassCannotExtendSealedType", new ProblemAttributes(CategorizedProblem.CAT_PREVIEW_RELATED));
 	    expectedProblemAttributes.put("SafeVarargsOnSyntheticRecordAccessor", new ProblemAttributes(true));
+	    expectedProblemAttributes.put("DiscouragedValueBasedTypeSynchronization", new ProblemAttributes(true));
 	    StringBuffer failures = new StringBuffer();
 		StringBuffer correctResult = new StringBuffer(70000);
 		Field[] fields = (iProblemClass = IProblem.class).getFields();
@@ -2349,6 +2350,7 @@
 	    expectedProblemAttributes.put("SealedLocalDirectSuperTypeSealed", SKIP);
 	    expectedProblemAttributes.put("SealedAnonymousClassCannotExtendSealedType", SKIP);
 	    expectedProblemAttributes.put("SafeVarargsOnSyntheticRecordAccessor", SKIP);
+	    expectedProblemAttributes.put("DiscouragedValueBasedTypeSynchronization", SKIP);
 	    Map constantNamesIndex = new HashMap();
 		Field[] fields = JavaCore.class.getFields();
 		for (int i = 0, length = fields.length; i < length; i++) {
diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/TestAll.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/TestAll.java
index 2d5d3d1..0f62c60 100644
--- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/TestAll.java
+++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/TestAll.java
@@ -209,6 +209,7 @@
 	 since_16.add(LocalEnumTest.class);
 	 since_16.add(LocalStaticsTest.class);
 	 since_16.add(PreviewFeatureTest.class);
+	 since_16.add(ValueBasedAnnotationTests.class);
 
 	 // Build final test suite
 	TestSuite all = new TestSuite(TestAll.class.getName());
diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ValueBasedAnnotationTests.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ValueBasedAnnotationTests.java
new file mode 100644
index 0000000..2ea38de
--- /dev/null
+++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ValueBasedAnnotationTests.java
@@ -0,0 +1,134 @@
+/*******************************************************************************
+ * Copyright (c) 2021 IBM Corporation and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *     IBM Corporation - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.jdt.core.tests.compiler.regression;
+import java.util.Map;
+import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
+import junit.framework.Test;
+public class ValueBasedAnnotationTests extends AbstractRegressionTest {
+	static {
+//		TESTS_NUMBERS = new int [] { 40 };
+//		TESTS_RANGE = new int[] { 1, -1 };
+//		TESTS_NAMES = new String[] { "testBug562219_001"};
+	}
+	public static Class<?> testClass() {
+		return ValueBasedAnnotationTests.class;
+	}
+	public static Test suite() {
+		return buildMinimalComplianceTestSuite(testClass(), F_16);
+	}
+	public ValueBasedAnnotationTests(String testName){
+		super(testName);
+	}
+	// Enables the tests to run individually
+	@Override
+	protected Map<String, String> getCompilerOptions() {
+		Map<String, String> defaultOptions = super.getCompilerOptions();
+		defaultOptions.put(CompilerOptions.OPTION_Compliance, CompilerOptions.VERSION_16);
+		defaultOptions.put(CompilerOptions.OPTION_Source, CompilerOptions.VERSION_16);
+		defaultOptions.put(CompilerOptions.OPTION_TargetPlatform, CompilerOptions.VERSION_16);
+		defaultOptions.put(CompilerOptions.OPTION_EnablePreviews, CompilerOptions.ENABLED);
+		defaultOptions.put(CompilerOptions.OPTION_ReportPreviewFeatures, CompilerOptions.IGNORE);
+		defaultOptions.put(CompilerOptions.OPTION_Store_Annotations, CompilerOptions.ENABLED);
+		return defaultOptions;
+	}
+	protected void runWarningTest(String[] testFiles, String expectedCompilerLog) {
+		runWarningTest(testFiles, expectedCompilerLog, null);
+	}
+	protected void runWarningTest(String[] testFiles, String expectedCompilerLog, Map<String, String> customOptions) {
+		runWarningTest(testFiles, expectedCompilerLog, customOptions, null);
+	}
+	protected void runWarningTest(String[] testFiles, String expectedCompilerLog,
+			Map<String, String> customOptions, String javacAdditionalTestOptions) {
+		if (!isJRE16Plus)
+			return;
+		Runner runner = new Runner();
+		runner.testFiles = testFiles;
+		runner.expectedCompilerLog = expectedCompilerLog;
+		runner.customOptions = customOptions;
+		runner.vmArguments = new String[] {"--enable-preview"};
+		runner.javacTestOptions = javacAdditionalTestOptions == null ? JavacTestOptions.forReleaseWithPreview("16") :
+			JavacTestOptions.forReleaseWithPreview("16", javacAdditionalTestOptions);
+		runner.runWarningTest();
+	}
+	protected void runConformTest(String[] testFiles) {
+		runConformTest(testFiles, (Map<String, String>)null, null);
+	}
+	protected void runConformTest(String[] testFiles, Map<String, String> customOptions, String javacAdditionalTestOptions) {
+		if (!isJRE16Plus)
+			return;
+		Runner runner = new Runner();
+		runner.testFiles = testFiles;
+		runner.customOptions = customOptions;
+		runner.vmArguments = new String[] {"--enable-preview"};
+		runner.javacTestOptions = javacAdditionalTestOptions == null ? JavacTestOptions.forReleaseWithPreview("16") :
+			JavacTestOptions.forReleaseWithPreview("16", javacAdditionalTestOptions);
+		runner.runConformTest();
+	}
+	public void testBug571507_001() {
+		this.runWarningTest(
+			new String[] {
+				"X.java",
+				"class X {\n" +
+				"  public static void main(String[] args){\n" +
+				"		Integer abc= Integer.valueOf(10);\n" +
+				"		synchronized(abc) {\n" +
+				"			\n" +
+				"		}" +
+				"  }\n"+
+				"}\n",
+			},
+			"----------\n" +
+			"1. WARNING in X.java (at line 4)\n" +
+			"	synchronized(abc) {\n" +
+			"	             ^^^\n" +
+			"Integer is a value-based type which is a discouraged argument for the synchronized statement\n" +
+			"----------\n");
+	}
+	public void testBug571507_002() {
+		this.runWarningTest(
+			new String[] {
+				"X.java",
+				"import java.util.Optional;\n\n" +
+				"class X {\n" +
+				"  public static void main(String[] args){\n" +
+				"		String[] sentence = new String[10];\n" +
+				"       Optional<String> abc = Optional.ofNullable(sentence[9]);  \n" +
+				"		synchronized (abc) { // no error given here.\n" +
+				"		}\n" +
+				"  }\n"+
+				"}\n",
+			},
+			"----------\n" +
+			"1. WARNING in X.java (at line 7)\n" +
+			"	synchronized (abc) { // no error given here.\n" +
+			"	              ^^^\n" +
+			"Optional<T> is a value-based type which is a discouraged argument for the synchronized statement\n" +
+			"----------\n");
+	}
+	public void testBug571507_003() {
+		this.runConformTest(
+			new String[] {
+				"X.java",
+				"import java.util.HashSet;\n\n" +
+				"class X {\n" +
+				"  public static void main(String[] args){\n" +
+				"		String[] sentence = new String[10];\n" +
+				"       HashSet<String> abc = new HashSet<>();  \n" +
+				"		synchronized (abc) { // no error given here.\n" +
+				"		}\n" +
+				"  }\n"+
+				"}\n",
+			});
+	}
+}
\ No newline at end of file
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/core/compiler/IProblem.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/core/compiler/IProblem.java
index 4b2c196..5da843c 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/core/compiler/IProblem.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/core/compiler/IProblem.java
@@ -2396,6 +2396,9 @@
 	 */
 	int PatternVariableRedeclared = Internal + 1784;
 
+	/** @since 3.26
+	 * @noreference */
+	int DiscouragedValueBasedTypeSynchronization = Internal + 1820;
 
 	/** @since 3.24
 	 * @noreference preview feature error */
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/SynchronizedStatement.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/SynchronizedStatement.java
index dbede23..9800b69 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/SynchronizedStatement.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/SynchronizedStatement.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2011, 2015 IBM Corporation and others.
+ * Copyright (c) 2000, 2021 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -201,6 +201,10 @@
 			case T_null :
 				this.scope.problemReporter().invalidNullToSynchronize(this.expression);
 				break;
+			default :
+				if (type.hasValueBasedTypeAnnotation()) {
+					this.scope.problemReporter().discouragedValueBasedTypeToSynchronize(this.expression, type);
+				}
 			}
 			//continue even on errors in order to have the TC done into the statements
 			this.synchroVariable = new LocalVariableBinding(SecretLocalDeclarationName, type, ClassFileConstants.AccDefault, false);
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/BinaryTypeBinding.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/BinaryTypeBinding.java
index b7d9e8b..da95cea 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/BinaryTypeBinding.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/BinaryTypeBinding.java
@@ -656,6 +656,9 @@
 		}
 		IBinaryAnnotation[] declAnnotations = binaryType.getAnnotations();
 		if (declAnnotations != null) {
+			if (hasValueBasedTypeAnnotation(declAnnotations)) {
+				this.extendedTagBits |= ExtendedTagBits.AnnotationValueBased;
+			}
 			for (IBinaryAnnotation annotation : declAnnotations) {
 				char[] typeName = annotation.getTypeName();
 				if (CharOperation.equals(typeName, ConstantPool.JDK_INTERNAL_PREVIEW_FEATURE)) {
@@ -734,6 +737,27 @@
 	return new TypeAnnotationWalker(annotations);
 }
 
+private boolean hasValueBasedTypeAnnotation(IBinaryAnnotation[] declAnnotations) {
+	boolean hasValueBasedAnnotation = false;
+	if (declAnnotations != null && declAnnotations.length > 0) {
+		for (IBinaryAnnotation annot : declAnnotations) {
+			char[] typeName= annot.getTypeName();
+			if ( typeName == null || typeName.length < 25 || typeName[0] != 'L')
+				continue;
+			char[][] name = CharOperation.splitOn('/', typeName, 1, typeName.length-1);
+			try {
+				if (CharOperation.equals(name,TypeConstants.JDK_INTERNAL_VALUEBASED)) {
+					hasValueBasedAnnotation= true;
+					break;
+				}
+			} catch (Exception e) {
+				//do nothing
+			}
+		}
+	}
+	return hasValueBasedAnnotation;
+}
+
 private int getNullDefaultFrom(IBinaryAnnotation[] declAnnotations) {
 	int result = 0;
 	if (declAnnotations != null) {
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ExtendedTagBits.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ExtendedTagBits.java
index ca6d15d..9743f8f 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ExtendedTagBits.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ExtendedTagBits.java
@@ -22,4 +22,8 @@
 
 	long AnnotationPreviewFeature = ASTNode.Bit32L | ASTNode.Bit33L;
 	long EssentialAPI = ASTNode.Bit11;
+	/** From Java 16
+	 *  Flag used to identify the annotation jdk.internal.ValueBased
+	 */
+	int AnnotationValueBased = ASTNode.Bit3;
 }
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java
index 2eaa03c..2306913 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java
@@ -875,6 +875,11 @@
 		return this.type.hasMethodWithNumArgs(selector, numArgs);
 	}
 
+	@Override
+	public boolean hasValueBasedTypeAnnotation() {
+		return this.type.hasValueBasedTypeAnnotation();
+	}
+
 	/**
 	 * @see org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding#implementsMethod(MethodBinding)
 	 */
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeBinding.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeBinding.java
index b85a71b..1029d70 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeBinding.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeBinding.java
@@ -1556,6 +1556,10 @@
 	return (this.tagBits & TagBits.HasTypeAnnotations) != 0;
 }
 
+public boolean hasValueBasedTypeAnnotation() {
+	return (this.extendedTagBits & ExtendedTagBits.AnnotationValueBased) != 0;
+}
+
 /**
  * Answer the qualified name of the receiver's package separated by periods
  * or an empty string if its the default package.
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeConstants.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeConstants.java
index 0cdddcb..0ab6af2 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeConstants.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/TypeConstants.java
@@ -545,4 +545,5 @@
 	char[] JAVA_BASE = "java.base".toCharArray(); //$NON-NLS-1$
 	String META_INF_MANIFEST_MF = "META-INF/MANIFEST.MF"; //$NON-NLS-1$
 	String AUTOMATIC_MODULE_NAME = "Automatic-Module-Name";  //$NON-NLS-1$
+	char[][] JDK_INTERNAL_VALUEBASED = {"jdk".toCharArray(), "internal".toCharArray(), "ValueBased".toCharArray()}; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
 }
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 bfdd420..c71769f 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
@@ -190,6 +190,7 @@
 import org.eclipse.jdt.internal.compiler.lookup.ModuleBinding;
 import org.eclipse.jdt.internal.compiler.lookup.PackageBinding;
 import org.eclipse.jdt.internal.compiler.lookup.ParameterizedGenericMethodBinding;
+import org.eclipse.jdt.internal.compiler.lookup.ParameterizedTypeBinding;
 import org.eclipse.jdt.internal.compiler.lookup.ProblemMethodBinding;
 import org.eclipse.jdt.internal.compiler.lookup.ProblemReasons;
 import org.eclipse.jdt.internal.compiler.lookup.ProblemReferenceBinding;
@@ -1750,6 +1751,9 @@
 			return ProblemSeverities.Warning;
 		case IProblem.IllegalUseOfUnderscoreAsAnIdentifier:
 			return this.underScoreIsError ? ProblemSeverities.Error : ProblemSeverities.Warning;
+		// for Java 16
+		case IProblem.DiscouragedValueBasedTypeSynchronization:
+			return ProblemSeverities.Warning;
 	}
 	int irritant = getIrritant(problemID);
 	if (irritant != 0) {
@@ -5079,6 +5083,17 @@
 			first.sourceStart,
 			last.sourceEnd);
 }
+public void discouragedValueBasedTypeToSynchronize(Expression expression, TypeBinding type) {
+	if (type.isParameterizedType()) {
+		type =  ((ParameterizedTypeBinding)type).actualType();
+	}
+	this.handle(
+		IProblem.DiscouragedValueBasedTypeSynchronization,
+		new String[] {new String(type.readableName())},
+		new String[] {new String(type.shortReadableName())},
+		expression.sourceStart,
+		expression.sourceEnd);
+}
 public void isClassPathCorrect(char[][] wellKnownTypeName, CompilationUnitDeclaration compUnitDecl, Object location, boolean implicitAnnotationUse) {
 	// ProblemReporter is not designed to be reentrant. Just in case, we discovered a build path problem while we are already
 	// in the midst of reporting some other problem, save and restore reference context thereby mimicking a stack.
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/problem/messages.properties b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/problem/messages.properties
index 8e5ba3b..4f3410f 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/problem/messages.properties
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/problem/messages.properties
@@ -1089,6 +1089,8 @@
 1809 = Invalid provides class
 1810 = Invalid module qualification
 
+# Java 16
+1820 =  {0} is a value-based type which is a discouraged argument for the synchronized statement
 
 # Java 15 Preview - cont
 1850 = The class {1} with a sealed direct superclass or a sealed direct superinterface {0} should be declared either final, sealed, or non-sealed