Bug 525250: "Create local variable" offered outside of local scopes

When invoked from within 'isApplicable(IMarker)', 'isCodanProblem()' did
not work as expected, since it used a cached value that was only updated
in 'run(IMarker)'. The old API 'isCodanProblem()' has been marked as
deprecated and is replaced by 'isCodanProblem(IMarker)', which works
directly on the marker, instead of using a cached result.

Additionally, two new APIs in 'QuickFixTestCase', called
'calculateQuickFixApplicability()' and
'assertIsApplicableForAllMarkers(boolean)', are introduced. The former
can be used to record the applicability of the QuickFix under test for
every marker in the test code, while the latter provides a way to assert
on the applicability. For finer grained assertions,
'calculateQuickFixApplicability()' returns the calculated map.

Change-Id: I7c53fd26afefa37ff086559acea75a7a33ecd5d7
Signed-off-by: Felix Morgner <fmorgner@hsr.ch>

codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/QuickFixAddSemicolon.java

@@ -42,7 +42,7 @@ public class QuickFixAddSemicolon extends AbstractAstRewriteQuickFix {
 			return;
 		}
 		IASTNode astNode = null;
-		if (isCodanProblem())
+		if (isCodanProblem(marker))
 			return;
 		
 		// We need to back up in the file

codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/QuickFixCreateLocalVariable.java

@@ -44,7 +44,7 @@ public class QuickFixCreateLocalVariable extends AbstractAstRewriteQuickFix {
 			return;
 		}
 		IASTName astName;
-		if (isCodanProblem()) {
+		if (isCodanProblem(marker)) {
 			astName = getASTNameFromMarker(marker, ast);
 		} else {
 			astName = getAstNameFromProblemArgument(marker, ast, 0);
@@ -77,7 +77,7 @@ public class QuickFixCreateLocalVariable extends AbstractAstRewriteQuickFix {
 
 	@Override
 	public boolean isApplicable(IMarker marker) {
-		if (isCodanProblem()) {
+		if (isCodanProblem(marker)) {
 			String problemArgument = getProblemArgument(marker, 1);
 			return problemArgument.contains(":func"); //$NON-NLS-1$
 		}

codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/QuickFixRenameMember.java

@@ -42,7 +42,7 @@ public class QuickFixRenameMember extends AbstractAstRewriteQuickFix {
 			return;
 		}
 		IASTName astName;
-		if (isCodanProblem()) {
+		if (isCodanProblem(marker)) {
 			astName = getASTNameFromMarker(marker, ast);
 		} else {
 			astName = getAstNameFromProblemArgument(marker, ast, 1);

codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CaseBreakQuickFixFallthroughAttributeTest.java

@@ -7,10 +7,27 @@
  *******************************************************************************/
 package org.eclipse.cdt.codan.internal.checkers.ui.quickfix;
 
+import org.eclipse.cdt.codan.internal.checkers.CaseBreakChecker;
 import org.eclipse.cdt.codan.ui.AbstractCodanCMarkerResolution;
 
+@SuppressWarnings("restriction")
 public class CaseBreakQuickFixFallthroughAttributeTest extends QuickFixTestCase {
-	@SuppressWarnings("restriction")
+
+	private boolean wasEnabled;
+
+	@Override
+	public void setUp() throws Exception {
+		super.setUp();
+		wasEnabled = (boolean) getPreference(CaseBreakChecker.ER_ID, CaseBreakChecker.PARAM_ENABLE_FALLTHROUGH_QUICKFIX).getValue();
+		setPreferenceValue(CaseBreakChecker.ER_ID, CaseBreakChecker.PARAM_ENABLE_FALLTHROUGH_QUICKFIX, true);
+	}
+
+	@Override
+	public void tearDown() throws Exception {
+		setPreferenceValue(CaseBreakChecker.ER_ID, CaseBreakChecker.PARAM_ENABLE_FALLTHROUGH_QUICKFIX, wasEnabled);
+		super.tearDown();
+	}
+
 	@Override
 	protected AbstractCodanCMarkerResolution createQuickFix() {
 		return new CaseBreakQuickFixFallthroughAttribute();

codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CreateLocalVariableQuickFixTest.java

@@ -98,4 +98,22 @@ public class CreateLocalVariableQuickFixTest extends QuickFixTestCase {
 		String result = runQuickFixOneFile();
 		assertContainedIn("char aChar", result); //$NON-NLS-1$
 	}
+
+	//int global = undefined;
+	public void testIsNotApplicableInGlobalScope_525250() throws Exception {
+		loadcode(getAboveComment());
+		indexFiles();
+		calculateQuickFixApplicability();
+		assertIsApplicableForAllMarkers(false);
+	}
+
+	//class Test{
+	//  int mem = var;
+	//};
+	public void testIsNotApplicableInClassScope_525250() throws Exception {
+		loadcode(getAboveComment());
+		indexFiles();
+		calculateQuickFixApplicability();
+		assertIsApplicableForAllMarkers(false);
+	}
 }

codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/QuickFixTestCase.java

@@ -11,6 +11,8 @@
 package org.eclipse.cdt.codan.internal.checkers.ui.quickfix;
 
 import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
 
 import org.eclipse.cdt.codan.core.PreferenceConstants;
 import org.eclipse.cdt.codan.core.tests.CheckerTestCase;
@@ -35,6 +37,7 @@ import org.eclipse.ui.PlatformUI;
 public abstract class QuickFixTestCase extends CheckerTestCase {
 	AbstractCodanCMarkerResolution quickFix;
 	Display display;
+	Map<IMarker, Boolean> isApplicableMap;
 
 	/**
 	 * Dispatch ui events for at least msec - milliseconds
@@ -73,6 +76,7 @@ public abstract class QuickFixTestCase extends CheckerTestCase {
 		super.setUp();
 		quickFix = createQuickFix();
 		display = PlatformUI.getWorkbench().getDisplay();
+		isApplicableMap = new HashMap<>();
 		closeWelcome();
 		IPreferenceStore store = CodanUIActivator.getDefault().getPreferenceStore(cproject.getProject());
 		// turn off editor reconciler
@@ -109,6 +113,22 @@ public abstract class QuickFixTestCase extends CheckerTestCase {
 		return new TextSelection(code.indexOf(string), string.length());
 	}
 
+	/**
+	 * Calculate for which markers in the current test the QuickFix under test is
+	 * applicable.
+	 *
+	 * @return A map reflecting for which markers the QuickFix is applicable.
+	 */
+	public Map<IMarker, Boolean> calculateQuickFixApplicability() {
+		runCodan();
+		Display.getDefault().syncExec(() -> {
+			for (IMarker marker : markers) {
+				isApplicableMap.put(marker, quickFix.isApplicable(marker));
+			}
+		});
+		return isApplicableMap;
+	}
+
 	public String runQuickFixOneFile() throws IOException, CoreException {
 		// need to load before running codan because otherwise marker is lost when doing quick fix 8[]
 		runCodan();
@@ -123,8 +143,11 @@ public abstract class QuickFixTestCase extends CheckerTestCase {
 			public void run() {
 				for (int i = 0; i < markers.length; i++) {
 					IMarker marker = markers[i];
-					quickFix.run(marker);
+					isApplicableMap.put(marker, quickFix.isApplicable(marker));
-					dispatch(0);
+					if (quickFix.isApplicable(marker)) {
+						quickFix.run(marker);
+						dispatch(0);
+					}
 				}
 				PlatformUI.getWorkbench().saveAllEditors(false);
 			}
@@ -140,6 +163,16 @@ public abstract class QuickFixTestCase extends CheckerTestCase {
 		assertTrue("Text <" + expected + "> not found in <" + result + ">", result.contains(expected)); //$NON-NLS-1$//$NON-NLS-2$ //$NON-NLS-3$
 	}
 
+	/**
+	 * Assert whether or not the QuickFix under test was applicable for all markers
+	 * in the test code.
+	 */
+	public void assertIsApplicableForAllMarkers(boolean expected) {
+		for (IMarker marker : markers) {
+			assertEquals(expected, (boolean) isApplicableMap.get(marker));
+		}
+	}
+
 	/**
 	 * Changes the quick fix to be used
 	 * @param quickFix

codan/org.eclipse.cdt.codan.ui.cxx/META-INF/MANIFEST.MF

@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %Bundle-Name
 Bundle-SymbolicName: org.eclipse.cdt.codan.ui.cxx;singleton:=true
-Bundle-Version: 3.3.0.qualifier
+Bundle-Version: 3.4.0.qualifier
 Bundle-Activator: org.eclipse.cdt.codan.internal.ui.cxx.Activator
 Bundle-Vendor: %Bundle-Vendor
 Require-Bundle: org.eclipse.ui,

codan/org.eclipse.cdt.codan.ui.cxx/src/org/eclipse/cdt/codan/ui/AbstractCodanCMarkerResolution.java

@@ -75,6 +75,20 @@ public abstract class AbstractCodanCMarkerResolution implements ICodanMarkerReso
 		return position;
 	}
 
+	/**
+	 * Check if the given marker is associated with a Codan problem
+	 * @since 3.4
+	 */
+	public boolean isCodanProblem(IMarker marker) {
+		return getProblemId(marker) != null;
+	}
+
+	/**
+	 * @deprecated Does not work as expected when called
+	 *             in {@link #isApplicable(IMarker)}, use
+	 *             {@link #isCodanProblem(IMarker)} instead
+	 */
+	@Deprecated
 	public boolean isCodanProblem() {
 		return codanProblem;
 	}