Bug 495842: Add a quick fix for codan problems that ignores them by adding a @suppress comment

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

codan/org.eclipse.cdt.codan.checkers.ui/plugin.xml

@@ -79,5 +79,8 @@
              class="org.eclipse.cdt.codan.internal.checkers.ui.quickfix.CaseBreakQuickFixFallthroughAttribute"
              problemId="org.eclipse.cdt.codan.internal.checkers.CaseBreakProblem">
        </resolution>
+       <universalResolution
+             class="org.eclipse.cdt.codan.internal.checkers.ui.quickfix.QuickFixSuppressProblem">
+       </universalResolution>
     </extension>
 </plugin>

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

@@ -25,6 +25,7 @@ public class QuickFixMessages extends NLS {
 	public static String QuickFixUsePointer_replace_dot;
 	public static String QuickFixUseDotOperator_replace_ptr;
 	public static String QuickFixForFixit_apply_fixit;
+	public static String QuickFixSuppressProblem_Label;
 
 	static {
 		NLS.initializeMessages(QuickFixMessages.class.getName(), QuickFixMessages.class);

codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/QuickFixMessages.properties

@@ -19,4 +19,5 @@ QuickFixRenameMember_rename_member=Rename to suggested member
 QuickFixAddSemicolon_add_semicolon=Add semicolon
 QuickFixUsePointer_replace_dot=Replace '.' with '->'
 QuickFixUseDotOperator_replace_ptr=Replace '->' with '.'
-QuickFixForFixit_apply_fixit=Apply compiler recommended fix-it
+QuickFixForFixit_apply_fixit=Apply compiler recommended fix-it
+QuickFixSuppressProblem_Label=Suppress problem "%s"

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

@@ -0,0 +1,75 @@
+/*******************************************************************************
+ * Copyright (c) 2017 Institute for Software.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Felix Morgner <fmorgner@hsr.ch> - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.cdt.codan.internal.checkers.ui.quickfix;
+
+import org.eclipse.cdt.codan.core.model.IProblem;
+import org.eclipse.cdt.codan.core.param.IProblemPreference;
+import org.eclipse.cdt.codan.core.param.RootProblemPreference;
+import org.eclipse.cdt.codan.core.param.SuppressionCommentProblemPreference;
+import org.eclipse.cdt.codan.internal.checkers.ui.CheckersUiActivator;
+import org.eclipse.cdt.codan.ui.AbstractAstRewriteQuickFix;
+import org.eclipse.cdt.codan.ui.ICodanMarkerResolutionExtension;
+import org.eclipse.cdt.core.index.IIndex;
+import org.eclipse.core.resources.IMarker;
+import org.eclipse.jface.text.BadLocationException;
+import org.eclipse.jface.text.IRegion;
+import org.eclipse.text.edits.InsertEdit;
+
+public class QuickFixSuppressProblem extends AbstractAstRewriteQuickFix implements ICodanMarkerResolutionExtension {
+	private static final String COMMENT_TEMPLATE = " // %s"; //$NON-NLS-1$
+	private String problemName;
+
+	@Override
+	public String getLabel() {
+		return String.format(QuickFixMessages.QuickFixSuppressProblem_Label, problemName);
+	}
+
+	@Override
+	public void modifyAST(IIndex index, IMarker marker) {
+		String supressionComment = getSupressionComment(getProblem(marker));
+		if (supressionComment == null) {
+			return;
+		}
+		try {
+			int line = marker.getAttribute(IMarker.LINE_NUMBER, 0);
+			IRegion lineInformation = getDocument().getLineInformation(line - 1);
+			int offset = lineInformation.getOffset() + lineInformation.getLength();
+			String commentString = String.format(COMMENT_TEMPLATE, supressionComment);
+			InsertEdit edit = new InsertEdit(offset, commentString);
+			edit.apply(getDocument());
+		} catch (BadLocationException e) {
+			CheckersUiActivator.log(e);
+		}
+	}
+
+	private String getSupressionComment(IProblem problem) {
+		IProblemPreference preference = problem.getPreference();
+		if (preference instanceof RootProblemPreference) {
+			RootProblemPreference root = (RootProblemPreference) preference;
+			Object value = root.getChildValue(SuppressionCommentProblemPreference.KEY);
+			if (value instanceof String && ((String) value).trim().length() > 0) {
+				return (String) value;
+			}
+		}
+		return null;
+	}
+
+	@Override
+	public void prepareFor(IMarker marker) {
+		problemName = getProblem(marker).getName();
+	}
+
+	@Override
+	public boolean isApplicable(IMarker marker) {
+		return getProblem(marker) != null;
+	}
+
+}

codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/tests/AutomatedIntegrationSuite.java

@@ -34,6 +34,7 @@ import org.eclipse.cdt.codan.internal.checkers.ui.quickfix.CaseBreakQuickFixComm
 import org.eclipse.cdt.codan.internal.checkers.ui.quickfix.CaseBreakQuickFixFallthroughAttributeTest;
 import org.eclipse.cdt.codan.internal.checkers.ui.quickfix.CatchByReferenceQuickFixTest;
 import org.eclipse.cdt.codan.internal.checkers.ui.quickfix.CreateLocalVariableQuickFixTest;
+import org.eclipse.cdt.codan.internal.checkers.ui.quickfix.QuickFixSuppressProblemTest;
 import org.eclipse.cdt.codan.internal.checkers.ui.quickfix.SuggestedParenthesisQuickFixTest;
 
 import junit.framework.Test;
@@ -86,6 +87,7 @@ public class AutomatedIntegrationSuite extends TestSuite {
 		suite.addTestSuite(CaseBreakQuickFixCommentTest.class);
 		suite.addTestSuite(CaseBreakQuickFixFallthroughAttributeTest.class);
 		suite.addTestSuite(AssignmentInConditionQuickFixTest.class);
+		suite.addTestSuite(QuickFixSuppressProblemTest.class);
 		return suite;
 	}
 }

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

@@ -0,0 +1,72 @@
+/*******************************************************************************
+ * Copyright (c) 2017 Institute for Software.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Felix Morgner <fmorgner@hsr.ch> - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.cdt.codan.internal.checkers.ui.quickfix;
+
+import java.io.BufferedWriter;
+import java.io.FileWriter;
+
+import org.eclipse.cdt.codan.ui.AbstractCodanCMarkerResolution;
+import org.eclipse.cdt.ui.PreferenceConstants;
+
+public class QuickFixSuppressProblemTest extends QuickFixTestCase {
+	@SuppressWarnings("restriction")
+	@Override
+	protected AbstractCodanCMarkerResolution createQuickFix() {
+		return new QuickFixSuppressProblem();
+	}
+
+	//struct s {};
+	//void func() {
+	//	try {
+	//	} catch (s e) {
+	//	}
+	//}
+	public void testCPPMarkerOnNode_495842() throws Exception {
+		loadcode(getAboveComment(), true);
+		String result = runQuickFixOneFile();
+		assertContainedIn("} catch (s e) { // @suppress(\"Catching by reference is recommended\")", result);
+	}
+
+	//void func() {
+	//	int n = 42;
+	//
+	//	switch (n) {
+	//	case 1:
+	//		n = 32;
+	//	default:
+	//		break;
+	//	}
+	//}
+	public void testCPPMarkerNotOnNode_495842() throws Exception {
+		loadcode(getAboveComment(), true);
+		String result = runQuickFixOneFile();
+		assertContainedIn("n = 32; // @suppress(\"No break at end of case\")", result);
+	}
+
+	//int func() { }
+	public void testCMarker_495842() throws Exception {
+		loadcode(getAboveComment(), false);
+		String result = runQuickFixOneFile();
+		assertContainedIn("int func() { } // @suppress(\"No return\")", result);
+	}
+
+	//int func() { }
+	public void testMarkerOnLastLineNoNewline_495842() throws Exception {
+		try(BufferedWriter writer = new BufferedWriter(new FileWriter(loadcode("", false)))) {
+			writer.write(getAboveComment().trim());
+		}
+		PreferenceConstants.getPreferenceStore().setValue(PreferenceConstants.ENSURE_NEWLINE_AT_EOF, false);
+		indexFiles();
+		String result = runQuickFixOneFile();
+		PreferenceConstants.getPreferenceStore().setValue(PreferenceConstants.ENSURE_NEWLINE_AT_EOF, true);
+		assertContainedIn("int func() { } // @suppress(\"No return\")", result);
+	}
+}

codan/org.eclipse.cdt.codan.ui/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; singleton:=true
-Bundle-Version: 3.2.1.qualifier
+Bundle-Version: 3.3.0.qualifier
 Bundle-Activator: org.eclipse.cdt.codan.internal.ui.CodanUIActivator
 Bundle-Vendor: %Bundle-Vendor
 Require-Bundle: org.eclipse.cdt.codan.core,

codan/org.eclipse.cdt.codan.ui/schema/codanMarkerResolution.exsd

@@ -19,6 +19,7 @@
       <complexType>
          <sequence minOccurs="1" maxOccurs="unbounded">
             <element ref="resolution"/>
+            <element ref="universalResolution"/>
          </sequence>
          <attribute name="point" type="string" use="required">
             <annotation>
@@ -88,6 +89,26 @@ If this is not codan problem (for example gcc error), it can be ommitted. Messag
       </complexType>
    </element>
 
+   <element name="universalResolution">
+      <annotation>
+         <documentation>
+            A resolution that is applicable to all markers
+         </documentation>
+      </annotation>
+      <complexType>
+         <attribute name="class" type="string">
+            <annotation>
+               <documentation>
+                  A class that implements IMarkerResolution, providing a fix applicable to any problem
+               </documentation>
+               <appinfo>
+                  <meta.attribute kind="java" basedOn=":org.eclipse.ui.IMarkerResolution"/>
+               </appinfo>
+            </annotation>
+         </attribute>
+      </complexType>
+   </element>
+
    <annotation>
       <appinfo>
          <meta.section type="since"/>

codan/org.eclipse.cdt.codan.ui/src/org/eclipse/cdt/codan/internal/ui/CodanProblemMarkerResolutionGenerator.java

@@ -14,14 +14,16 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.Iterator;
+import java.util.List;
 import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
 
 import org.eclipse.cdt.codan.core.model.ICodanProblemMarker;
 import org.eclipse.cdt.codan.internal.core.model.CodanProblemMarker;
 import org.eclipse.cdt.codan.ui.ICodanMarkerResolution;
+import org.eclipse.cdt.codan.ui.ICodanMarkerResolutionExtension;
 import org.eclipse.core.resources.IMarker;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IConfigurationElement;
@@ -32,16 +34,68 @@ import org.eclipse.ui.IMarkerResolutionGenerator;
 
 public class CodanProblemMarkerResolutionGenerator implements IMarkerResolutionGenerator {
 	private static final String EXTENSION_POINT_NAME = "codanMarkerResolution"; //$NON-NLS-1$
-	private static final Map<String, Collection<ConditionalResolution>> resolutions = new HashMap<String, Collection<ConditionalResolution>>();
+	private static final Map<String, Collection<ConditionalResolution>> conditionalResolutions = new HashMap<>();
+	private static final List<IMarkerResolution> universalResolutions = new ArrayList<>();
 	private static boolean resolutionsLoaded;
 
 	static class ConditionalResolution {
-		IMarkerResolution res;
+		private final Pattern messagePattern;
-		String messagePattern;
+		private final IMarkerResolution resolutionInstance;
 
-		public ConditionalResolution(IMarkerResolution res, String messagePattern) {
+		public static ConditionalResolution createFrom(IConfigurationElement configurationElement) {
-			this.res = res;
+			String rawPattern = configurationElement.getAttribute("messagePattern"); //$NON-NLS-1$
+			try {
+				return new ConditionalResolution(configurationElement, rawPattern != null ? Pattern.compile(rawPattern) : null);
+			} catch (PatternSyntaxException e) {
+				CodanUIActivator.log("Invalid message pattern: " + rawPattern); //$NON-NLS-1$
+			}
+			return null;
+		}
+
+		private ConditionalResolution(IConfigurationElement resolutionElement, Pattern messagePattern) {
 			this.messagePattern = messagePattern;
+			this.resolutionInstance = instantiateResolution(resolutionElement);
+		}
+
+		public boolean isApplicableFor(IMarker marker) {
+			if (resolutionInstance instanceof ICodanMarkerResolution) {
+				if(!((ICodanMarkerResolution) resolutionInstance).isApplicable(marker)) {
+					return false;
+				}
+			}
+
+			return messagePattern == null || messagePattern.matcher(marker.getAttribute(IMarker.MESSAGE, "")).matches(); //$NON-NLS-1$
+		}
+
+		public IMarkerResolution getResolution() {
+			return resolutionInstance;
+		}
+
+		public Pattern getMessagePattern() {
+			return messagePattern;
+		}
+
+		public void setMarkerArguments(IMarker marker) {
+			if (messagePattern == null) {
+				return;
+			}
+			String message = marker.getAttribute(IMarker.MESSAGE, ""); //$NON-NLS-1$
+			Matcher matcher = messagePattern.matcher(message);
+			int n = matcher.groupCount();
+			if (n == 0)
+				return;
+			String[] res = new String[n];
+			for (int i = 0; i < n; i++) {
+				res[i] = matcher.group(i + 1);
+			}
+			String[] old = CodanProblemMarker.getProblemArguments(marker);
+			if (!Arrays.deepEquals(res, old)) {
+				try {
+					CodanProblemMarker.setProblemArguments(marker, res);
+				} catch (CoreException e) {
+					CodanUIActivator.log(e);
+				}
+			}
 		}
 	}
 
@@ -51,61 +105,35 @@ public class CodanProblemMarkerResolutionGenerator implements IMarkerResolutionG
 			readExtensions();
 		}
 		String id = marker.getAttribute(ICodanProblemMarker.ID, null);
-		if (id == null && resolutions.get(null) == null)
+		if (id == null && conditionalResolutions.get(id) == null)
 			return new IMarkerResolution[0];
-		String message = marker.getAttribute(IMarker.MESSAGE, ""); //$NON-NLS-1$
+
-		Collection<ConditionalResolution> collection = resolutions.get(id);
+		Collection<ConditionalResolution> candidates = conditionalResolutions.get(id);
-		if (collection != null) {
+		ArrayList<IMarkerResolution> resolutions = new ArrayList<IMarkerResolution>();
-			ArrayList<IMarkerResolution> list = new ArrayList<IMarkerResolution>();
+
-			for (Iterator<ConditionalResolution> iterator = collection.iterator(); iterator.hasNext();) {
+		if (candidates != null) {
-				ConditionalResolution res = iterator.next();
+			candidates.stream()
-				if (res.messagePattern != null) {
+				.filter(candidate -> candidate.isApplicableFor(marker))
-					try {
+				.peek(candidate -> candidate.setMarkerArguments(marker))
-						Pattern pattern = Pattern.compile(res.messagePattern);
+				.map(ConditionalResolution::getResolution)
-						Matcher matcher = pattern.matcher(message);
+				.forEach(resolutions::add);
-						if (!matcher.matches())
-							continue;
-						if (id == null) {
-							setArgumentsFromPattern(matcher, marker);
-						}
-					} catch (Exception e) {
-						CodanUIActivator.log("Cannot compile regex: " + res.messagePattern); //$NON-NLS-1$
-						continue;
-					}
-				}
-				if (res.res instanceof ICodanMarkerResolution) {
-					if (!((ICodanMarkerResolution) res.res).isApplicable(marker))
-						continue;
-				}
-				list.add(res.res);
-			}
-			if (list.size() > 0)
-				return list.toArray(new IMarkerResolution[list.size()]);
 		}
-		return new IMarkerResolution[0];
+
+		universalResolutions.stream()
+			.filter(res -> !(res instanceof ICodanMarkerResolution) || ((ICodanMarkerResolution) res).isApplicable(marker))
+			.forEach(resolutions::add);
+
+		return resolutions.stream().peek(res -> {
+			if(res instanceof ICodanMarkerResolutionExtension) {
+				((ICodanMarkerResolutionExtension) res).prepareFor(marker);
+			}
+		}).toArray(IMarkerResolution[]::new);
 	}
 
 	/**
 	 * @param matcher
 	 * @param marker
 	 */
-	private void setArgumentsFromPattern(Matcher matcher, IMarker marker) {
-		int n = matcher.groupCount();
-		if (n == 0)
-			return;
-		String[] res = new String[n];
-		for (int i = 0; i < n; i++) {
-			res[i] = matcher.group(i + 1);
-		}
-		String[] old = CodanProblemMarker.getProblemArguments(marker);
-		if (!Arrays.deepEquals(res, old)) {
-			try {
-				CodanProblemMarker.setProblemArguments(marker, res);
-			} catch (CoreException e) {
-				CodanUIActivator.log(e);
-			}
-		}
-	}
 
 	private static synchronized void readExtensions() {
 		IExtensionPoint ep = Platform.getExtensionRegistry().getExtensionPoint(CodanUIActivator.PLUGIN_ID, EXTENSION_POINT_NAME);
@@ -127,7 +155,8 @@ public class CodanProblemMarkerResolutionGenerator implements IMarkerResolutionG
 	 * @param configurationElement
 	 */
 	private static void processResolution(IConfigurationElement configurationElement) {
-		if (configurationElement.getName().equals("resolution")) { //$NON-NLS-1$
+		final String elementName = configurationElement.getName();
+		if (elementName.equals("resolution")) { //$NON-NLS-1$
 			String id = configurationElement.getAttribute("problemId"); //$NON-NLS-1$
 			String messagePattern = configurationElement.getAttribute("messagePattern"); //$NON-NLS-1$
 			if (id == null && messagePattern == null) {
@@ -135,38 +164,32 @@ public class CodanProblemMarkerResolutionGenerator implements IMarkerResolutionG
 						+ " problemId is not defined"); //$NON-NLS-1$
 				return;
 			}
-			IMarkerResolution res;
+			ConditionalResolution candidate = ConditionalResolution.createFrom(configurationElement);
-			try {
+			if (candidate == null) {
-				res = (IMarkerResolution) configurationElement.createExecutableExtension("class");//$NON-NLS-1$
-			} catch (CoreException e) {
-				CodanUIActivator.log(e);
 				return;
 			}
-			if (messagePattern != null) {
+			addResolution(id, candidate);
-				try {
+		}
-					Pattern.compile(messagePattern);
+		else if (elementName.equals("universalResolution")) { //$NON-NLS-1$
-				} catch (Exception e) {
+			universalResolutions.add(instantiateResolution(configurationElement));
-					// bad pattern log and ignore
-					CodanUIActivator.log("Extension for " //$NON-NLS-1$
-							+ EXTENSION_POINT_NAME + " messagePattern is invalid: " + e.getMessage()); //$NON-NLS-1$
-					return;
-				}
-			}
-			ConditionalResolution co = new ConditionalResolution(res, messagePattern);
-			addResolution(id, co);
 		}
 	}
 
-	public static void addResolution(String id, IMarkerResolution res, String messagePattern) {
+	private static IMarkerResolution instantiateResolution(IConfigurationElement element) {
-		addResolution(id, new ConditionalResolution(res, messagePattern));
+		try {
+			return  (IMarkerResolution) element.createExecutableExtension("class");//$NON-NLS-1$
+		} catch (CoreException e) {
+			CodanUIActivator.log(e);
+		}
+		return null;
 	}
 
 	private static void addResolution(String id, ConditionalResolution res) {
-		Collection<ConditionalResolution> collection = resolutions.get(id);
+		Collection<ConditionalResolution> candidates = conditionalResolutions.get(id);
-		if (collection == null) {
+		if (candidates == null) {
-			collection = new ArrayList<ConditionalResolution>();
+			candidates = new ArrayList<ConditionalResolution>();
-			resolutions.put(id, collection);
+			conditionalResolutions.put(id, candidates);
 		}
-		collection.add(res);
+		candidates.add(res);
 	}
 }

codan/org.eclipse.cdt.codan.ui/src/org/eclipse/cdt/codan/ui/ICodanMarkerResolutionExtension.java

@@ -0,0 +1,26 @@
+/*******************************************************************************
+ * Copyright (c) 2017 Institute for Software.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Felix Morgner <fmorgner@hsr.ch> - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.cdt.codan.ui;
+
+import org.eclipse.core.resources.IMarker;
+
+/**
+ * @since 3.3
+ */
+public interface ICodanMarkerResolutionExtension extends ICodanMarkerResolution {
+	/**
+	 * This method will be called before the fix is suggested to the user. It
+	 * enables customization based on the problem attached to the marker.
+	 *
+	 * @param marker
+	 */
+	public void prepareFor(IMarker marker);
+}