From 39be625d8e599210130fe8098a018fc62e52ef70 Mon Sep 17 00:00:00 2001 From: Marco Stornelli Date: Sat, 27 Jul 2019 12:41:41 +0200 Subject: [PATCH] Bug 549466 - Quick fix with the same resolution We have the "universal resolution" in an ArrayList and they are used as single instances. So what can happen here is that we add the same universal resolution instance multiple times for several markers. However this approach can't work because a "marker resolution" is designed to fix a single problem. Indeed the last marker analyzed override the previous one in the single "universal resolution" instance, so what we have is N proposals pointing to the same resolution where the problem description is just the latest one. To solve the problem we instantiate the universal resolution multiple times. Change-Id: I072ca0b4dabff9781d6230a218eeb7dd388c648d Signed-off-by: Marco Stornelli --- .../internal/checkers/ReturnStyleChecker.java | 2 +- .../ui/quickfix/QuickFixSuppressProblemTest.java | 14 ++++++++++++++ .../ui/CodanProblemMarkerResolutionGenerator.java | 6 +++--- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnStyleChecker.java b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnStyleChecker.java index 403b73b5ece..ae17a79870a 100644 --- a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnStyleChecker.java +++ b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnStyleChecker.java @@ -22,7 +22,7 @@ import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit; import org.eclipse.cdt.core.dom.ast.IASTUnaryExpression; public class ReturnStyleChecker extends AbstractIndexAstChecker { - public final String ERR_ID = "org.eclipse.cdt.codan.internal.checkers.ReturnStyleProblem"; //$NON-NLS-1$ + public static final String ERR_ID = "org.eclipse.cdt.codan.internal.checkers.ReturnStyleProblem"; //$NON-NLS-1$ @Override public boolean runInEditor() { diff --git a/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/QuickFixSuppressProblemTest.java b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/QuickFixSuppressProblemTest.java index 5a7d4dc68e4..ce86fe576f0 100644 --- a/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/QuickFixSuppressProblemTest.java +++ b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/QuickFixSuppressProblemTest.java @@ -16,10 +16,13 @@ package org.eclipse.cdt.codan.internal.checkers.ui.quickfix; import java.io.BufferedWriter; import java.io.FileWriter; +import org.eclipse.cdt.codan.internal.checkers.CommentChecker; +import org.eclipse.cdt.codan.internal.checkers.ReturnStyleChecker; import org.eclipse.cdt.codan.ui.AbstractCodanCMarkerResolution; import org.eclipse.cdt.ui.PreferenceConstants; public class QuickFixSuppressProblemTest extends QuickFixTestCase { + @SuppressWarnings("restriction") @Override protected AbstractCodanCMarkerResolution createQuickFix() { @@ -66,6 +69,17 @@ public class QuickFixSuppressProblemTest extends QuickFixTestCase { assertContainedIn("int func() { } // @suppress(\"No return\")", result); } + //int main() { + // return 0; //Line + //} + public void testMultipleSuppress_549466() throws Exception { + enableProblems(ReturnStyleChecker.ERR_ID, CommentChecker.COMMENT_NO_LINE); + loadcode(getAboveComment(), false); + String result = runQuickFixOneFile(); + assertContainedIn("// @suppress(\"Return with parenthesis\")", result); + assertContainedIn("// @suppress(\"Line comments\")", result); + } + //int func() { } public void testMarkerOnLastLineNoNewline_495842() throws Exception { try (BufferedWriter writer = new BufferedWriter(new FileWriter(loadcode("", false)))) { diff --git a/codan/org.eclipse.cdt.codan.ui/src/org/eclipse/cdt/codan/internal/ui/CodanProblemMarkerResolutionGenerator.java b/codan/org.eclipse.cdt.codan.ui/src/org/eclipse/cdt/codan/internal/ui/CodanProblemMarkerResolutionGenerator.java index 03406b67e03..895b9ffd9ca 100644 --- a/codan/org.eclipse.cdt.codan.ui/src/org/eclipse/cdt/codan/internal/ui/CodanProblemMarkerResolutionGenerator.java +++ b/codan/org.eclipse.cdt.codan.ui/src/org/eclipse/cdt/codan/internal/ui/CodanProblemMarkerResolutionGenerator.java @@ -38,7 +38,7 @@ 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> conditionalResolutions = new HashMap<>(); - private static final List universalResolutions = new ArrayList<>(); + private static final List universalResolutions = new ArrayList<>(); private static boolean resolutionsLoaded; static class ConditionalResolution { @@ -123,7 +123,7 @@ public class CodanProblemMarkerResolutionGenerator implements IMarkerResolutionG .forEach(resolutions::add); } - universalResolutions.stream().filter( + universalResolutions.stream().map(e -> instantiateResolution(e)).filter( res -> !(res instanceof ICodanMarkerResolution) || ((ICodanMarkerResolution) res).isApplicable(marker)) .forEach(resolutions::add); @@ -175,7 +175,7 @@ public class CodanProblemMarkerResolutionGenerator implements IMarkerResolutionG } addResolution(id, candidate); } else if (elementName.equals("universalResolution")) { //$NON-NLS-1$ - universalResolutions.add(instantiateResolution(configurationElement)); + universalResolutions.add(configurationElement); } }