diff options
author | Felix Morgner | 2018-03-07 15:14:20 +0000 |
---|---|---|
committer | Thomas Corbat | 2018-06-03 10:37:37 +0000 |
commit | 5ec251a79140995642053f3d27f2cc3fb218e3a9 (patch) | |
tree | 14dfa6aec658fe77151935998bffd7f5063b8a11 | |
parent | 5c5ce995f6eb390218a1ae5cdb17f23c15b3c6ef (diff) | |
download | org.eclipse.cdt-5ec251a79140995642053f3d27f2cc3fb218e3a9.tar.gz org.eclipse.cdt-5ec251a79140995642053f3d27f2cc3fb218e3a9.tar.xz org.eclipse.cdt-5ec251a79140995642053f3d27f2cc3fb218e3a9.zip |
Bug 532120: Catch by const reference ignores const placement setting
The original implementation used plain-text string manipulation of the
IDocument. This changeset changes the implementation to make use of the
ASTRewrite infrastructure, which automatically honors the const
placement setting.
Change-Id: Ib5ae9381b93ca8ab4d1ad3e16b1c3c8b1ec62d78
Signed-off-by: Felix Morgner <fmorgner@hsr.ch>
3 files changed, 138 insertions, 51 deletions
diff --git a/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CatchByConstReferenceQuickFix.java b/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CatchByConstReferenceQuickFix.java index 635d9ceb1d6..e949ef550a0 100644 --- a/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CatchByConstReferenceQuickFix.java +++ b/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CatchByConstReferenceQuickFix.java @@ -11,22 +11,26 @@ *******************************************************************************/ package org.eclipse.cdt.codan.internal.checkers.ui.quickfix; +import java.util.Optional; + import org.eclipse.cdt.codan.internal.checkers.ui.Messages; -import org.eclipse.cdt.codan.ui.AbstractCodanCMarkerResolution; -import org.eclipse.core.resources.IMarker; -import org.eclipse.jface.text.IDocument; +import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier; +import org.eclipse.cdt.core.dom.ast.IASTNode.CopyStyle; +import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration; /** * Quick fix for catch by value */ -public class CatchByConstReferenceQuickFix extends AbstractCodanCMarkerResolution { +public class CatchByConstReferenceQuickFix extends CatchByReferenceQuickFix { @Override public String getLabel() { return Messages.CatchByConstReferenceQuickFix_Message; } - @Override - public void apply(IMarker marker, IDocument document) { - CatchByReferenceQuickFix.applyCatchByReferenceQuickFix(marker, document, true); - } + protected Optional<IASTDeclSpecifier> getNewDeclSpecifier(IASTSimpleDeclaration declaration) { + IASTDeclSpecifier declSpecifier = declaration.getDeclSpecifier(); + IASTDeclSpecifier replacement = declSpecifier.copy(CopyStyle.withLocations); + replacement.setConst(true); + return Optional.of(replacement); + } } diff --git a/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CatchByReferenceQuickFix.java b/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CatchByReferenceQuickFix.java index 300b98506af..4986b52ef73 100644 --- a/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CatchByReferenceQuickFix.java +++ b/codan/org.eclipse.cdt.codan.checkers.ui/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CatchByReferenceQuickFix.java @@ -10,65 +10,97 @@ *******************************************************************************/ package org.eclipse.cdt.codan.internal.checkers.ui.quickfix; +import java.util.Optional; + import org.eclipse.cdt.codan.internal.checkers.ui.CheckersUiActivator; import org.eclipse.cdt.codan.internal.checkers.ui.Messages; -import org.eclipse.cdt.codan.ui.AbstractCodanCMarkerResolution; +import org.eclipse.cdt.codan.ui.AbstractAstRewriteQuickFix; +import org.eclipse.cdt.core.dom.ast.ASTNodeFactoryFactory; +import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier; +import org.eclipse.cdt.core.dom.ast.IASTDeclarator; +import org.eclipse.cdt.core.dom.ast.IASTNode; +import org.eclipse.cdt.core.dom.ast.IASTNode.CopyStyle; +import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration; +import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTReferenceOperator; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPNodeFactory; +import org.eclipse.cdt.core.dom.rewrite.ASTRewrite; +import org.eclipse.cdt.core.index.IIndex; +import org.eclipse.cdt.core.model.ITranslationUnit; import org.eclipse.core.resources.IMarker; -import org.eclipse.jface.text.BadLocationException; -import org.eclipse.jface.text.IDocument; +import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.NullProgressMonitor; /** * Quick fix for catch by value */ -public class CatchByReferenceQuickFix extends AbstractCodanCMarkerResolution { +public class CatchByReferenceQuickFix extends AbstractAstRewriteQuickFix { @Override public String getLabel() { return Messages.CatchByReferenceQuickFix_Message; } @Override - public void apply(IMarker marker, IDocument document) { - applyCatchByReferenceQuickFix(marker, document, false); - } + public void modifyAST(IIndex index, IMarker marker) { + IASTSimpleDeclaration declaration = getDeclaration(index, marker); + if (declaration == null) { + CheckersUiActivator.log("Could not find declaration"); //$NON-NLS-1$ + return; + } + + IASTTranslationUnit ast = declaration.getTranslationUnit(); + ASTRewrite rewrite = ASTRewrite.create(ast); + + getNewDeclSpecifier(declaration).ifPresent(ds -> rewrite.replace(declaration.getDeclSpecifier(), ds, null)); + rewrite.replace(declaration.getDeclarators()[0], getNewDeclarator(declaration), null); - static void applyCatchByReferenceQuickFix(IMarker marker, IDocument document, boolean addConst) { try { - int left = marker.getAttribute(IMarker.CHAR_START, -1); - int right = marker.getAttribute(IMarker.CHAR_END, -1); - String inStr = document.get(left, right - left); - document.replace(left, right - left, getCatchByReferenceString(inStr, addConst)); - } catch (BadLocationException e) { + rewrite.rewriteAST().perform(new NullProgressMonitor()); + } catch (CoreException e) { CheckersUiActivator.log(e); } } - + /** - * Returns a catch by reference string from a catch by value string + * Calculate the new IASTDeclSpecifier for the changed declaration + * <p> + * Subclasses can override this method to provide custom behavior. + * </p> + * + * @param declaration The original declaration + * @return A, possibly empty, {@link Optional} containing the new + * IASTDeclSpecifier */ - static private String getCatchByReferenceString(String inStr, boolean addConst) { - StringBuilder stringBuilder = new StringBuilder(inStr.length() + 10); - if (addConst) { - stringBuilder.append("const "); //$NON-NLS-1$ - } - - String typename; - int space = inStr.lastIndexOf(' '); - boolean hasDeclName = space != -1; - if (hasDeclName) { - typename = inStr.substring(0,space); - } else { - typename = inStr; - } - stringBuilder.append(typename); - - stringBuilder.append(" &"); //$NON-NLS-1$ - - if (hasDeclName) { - stringBuilder.append(" "); //$NON-NLS-1$ - String declname = inStr.substring(space+1); - stringBuilder.append(declname); + protected Optional<IASTDeclSpecifier> getNewDeclSpecifier(IASTSimpleDeclaration declaration) { + return Optional.empty(); + } + + private static IASTDeclarator getNewDeclarator(IASTSimpleDeclaration declaration) { + ICPPNodeFactory nodeFactory = ASTNodeFactoryFactory.getDefaultCPPNodeFactory(); + ICPPASTReferenceOperator reference = nodeFactory.newReferenceOperator(false); + IASTDeclarator declarator = declaration.getDeclarators()[0]; + IASTDeclarator replacement = declarator.copy(CopyStyle.withLocations); + replacement.addPointerOperator(reference); + return replacement; + } + + private IASTSimpleDeclaration getDeclaration(IIndex index, IMarker marker) { + try { + ITranslationUnit tu = getTranslationUnitViaEditor(marker); + IASTTranslationUnit ast = tu.getAST(index, ITranslationUnit.AST_SKIP_ALL_HEADERS); + int start = marker.getAttribute(IMarker.CHAR_START, -1); + int end = marker.getAttribute(IMarker.CHAR_END, -1); + if (start != -1 && end != -1) { + IASTNode node = ast.getNodeSelector(null).findNode(start, end - start); + while (node != null && !(node instanceof IASTSimpleDeclaration)) { + node = node.getParent(); + } + return (IASTSimpleDeclaration) node; + } + } catch (CoreException e) { + CheckersUiActivator.log(e); } - - return stringBuilder.toString(); + return null; } + } diff --git a/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CatchByReferenceQuickFixTest.java b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CatchByReferenceQuickFixTest.java index d3dd2f85391..171f87c0246 100644 --- a/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CatchByReferenceQuickFixTest.java +++ b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/internal/checkers/ui/quickfix/CatchByReferenceQuickFixTest.java @@ -12,6 +12,10 @@ package org.eclipse.cdt.codan.internal.checkers.ui.quickfix; import org.eclipse.cdt.codan.internal.checkers.CatchByReference; import org.eclipse.cdt.codan.ui.AbstractCodanCMarkerResolution; +import org.eclipse.cdt.core.CCorePlugin; +import org.eclipse.cdt.core.CCorePreferenceConstants; +import org.eclipse.core.resources.ProjectScope; +import org.eclipse.core.runtime.preferences.IEclipsePreferences; /** * @author Tomasz Wesolowski @@ -29,6 +33,16 @@ public class CatchByReferenceQuickFixTest extends QuickFixTestCase { return true; } + private boolean setPlaceConstRight(boolean placeRight) { + IEclipsePreferences node = new ProjectScope(cproject.getProject()).getNode(CCorePlugin.PLUGIN_ID); + boolean before = node.getBoolean(CCorePreferenceConstants.PLACE_CONST_RIGHT_OF_TYPE, + CCorePreferenceConstants.DEFAULT_PLACE_CONST_RIGHT_OF_TYPE); + node.putBoolean(CCorePreferenceConstants.PLACE_CONST_RIGHT_OF_TYPE, placeRight); + CCorePreferenceConstants.getPreference(CCorePreferenceConstants.PLACE_CONST_RIGHT_OF_TYPE, cproject, + CCorePreferenceConstants.DEFAULT_PLACE_CONST_RIGHT_OF_TYPE); + return before; + } + @Override protected AbstractCodanCMarkerResolution createQuickFix() { return null; // quick fix to be chosen per test @@ -45,7 +59,7 @@ public class CatchByReferenceQuickFixTest extends QuickFixTestCase { setQuickFix(new CatchByReferenceQuickFix()); loadcode(getAboveComment()); String result = runQuickFixOneFile(); - assertContainedIn("catch (C & exception)", result); //$NON-NLS-1$ + assertContainedIn("catch (C &exception)", result); //$NON-NLS-1$ } // struct C { @@ -59,7 +73,7 @@ public class CatchByReferenceQuickFixTest extends QuickFixTestCase { setQuickFix(new CatchByReferenceQuickFix()); loadcode(getAboveComment()); String result = runQuickFixOneFile(); - assertContainedIn("catch (C &)", result); //$NON-NLS-1$ + assertContainedIn("catch (C&)", result); //$NON-NLS-1$ } // struct C { @@ -73,7 +87,25 @@ public class CatchByReferenceQuickFixTest extends QuickFixTestCase { setQuickFix(new CatchByConstReferenceQuickFix()); loadcode(getAboveComment()); String result = runQuickFixOneFile(); - assertContainedIn("catch (const C & exception)", result); //$NON-NLS-1$ + assertContainedIn("catch (const C &exception)", result); //$NON-NLS-1$ + } + + // struct C { + // }; + // void foo() { + // try { + // } catch (C exception) { + // } + // } + public void testCatchByConstReferenceHonorsConstPlacementSettings_532120() throws Exception { + setQuickFix(new CatchByConstReferenceQuickFix()); + loadcode(getAboveComment()); + + boolean before = setPlaceConstRight(true); + String result = runQuickFixOneFile(); + setPlaceConstRight(before); + + assertContainedIn("catch (C const &exception)", result); //$NON-NLS-1$ } // struct C { @@ -87,6 +119,25 @@ public class CatchByReferenceQuickFixTest extends QuickFixTestCase { setQuickFix(new CatchByConstReferenceQuickFix()); loadcode(getAboveComment()); String result = runQuickFixOneFile(); - assertContainedIn("catch (const C &)", result); //$NON-NLS-1$ + assertContainedIn("catch (const C&)", result); //$NON-NLS-1$ } + + // struct C { + // }; + // void foo() { + // try { + // } catch (C) { + // } + // } + public void testCatchByConstReferenceNoDeclNameHonorsConstPlacementSettings_532120() throws Exception { + setQuickFix(new CatchByConstReferenceQuickFix()); + loadcode(getAboveComment()); + + boolean before = setPlaceConstRight(true); + String result = runQuickFixOneFile(); + setPlaceConstRight(before); + + assertContainedIn("catch (C const &)", result); //$NON-NLS-1$ + } + } |