From 1610de526e564e30991f6067543dbf4d4d7df0f1 Mon Sep 17 00:00:00 2001 From: Sergey Prigogin Date: Wed, 4 Jan 2012 17:54:50 -0800 Subject: [PATCH 1/5] Cosmetics. --- .../org/eclipse/cdt/internal/ui/refactoring/ChangeTreeSet.java | 2 +- .../src/org/eclipse/cdt/internal/ui/refactoring/Container.java | 2 +- .../gettersandsetters/GenerateGettersAndSettersRefactoring.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/ChangeTreeSet.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/ChangeTreeSet.java index 04ef64201b5..44b9e7cae05 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/ChangeTreeSet.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/ChangeTreeSet.java @@ -27,7 +27,7 @@ public class ChangeTreeSet { private static final class ChangePositionComparator implements Comparator { @Override public int compare(CTextFileChange o1, CTextFileChange o2) { - if(o1.getFile().equals(o2.getFile())){ + if (o1.getFile().equals(o2.getFile())) { return o2.getEdit().getOffset() - o1.getEdit().getOffset(); } return o2.getFile().hashCode() - o1.getFile().hashCode(); diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/Container.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/Container.java index e753f256287..29eb8b5220f 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/Container.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/Container.java @@ -7,7 +7,7 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * Institute for Software - initial API and implementation + * Institute for Software - initial API and implementation *******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring; diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GenerateGettersAndSettersRefactoring.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GenerateGettersAndSettersRefactoring.java index 7f6d0c36d2b..f1f3e6794e5 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GenerateGettersAndSettersRefactoring.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GenerateGettersAndSettersRefactoring.java @@ -226,7 +226,7 @@ public class GenerateGettersAndSettersRefactoring extends CRefactoring2 { } @Override - protected void collectModifications(IProgressMonitor pm,ModificationCollector collector) + protected void collectModifications(IProgressMonitor pm, ModificationCollector collector) throws CoreException, OperationCanceledException { List getterAndSetters = new ArrayList(); List definitions = new ArrayList(); From 6da58da8f81e691cc923fc5b76744cc72864baaf Mon Sep 17 00:00:00 2001 From: Sergey Prigogin Date: Wed, 4 Jan 2012 20:05:07 -0800 Subject: [PATCH 2/5] Code streamlining. --- .../AddDeclarationNodeToClassChange.java | 32 +++++++------------ 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/AddDeclarationNodeToClassChange.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/AddDeclarationNodeToClassChange.java index 0fa86ca0b49..2cd2b770f05 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/AddDeclarationNodeToClassChange.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/AddDeclarationNodeToClassChange.java @@ -11,7 +11,7 @@ *******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring; -import java.util.ArrayList; +import java.util.Collections; import java.util.List; import org.eclipse.osgi.util.NLS; @@ -40,37 +40,27 @@ import org.eclipse.cdt.internal.ui.refactoring.utils.VisibilityEnum; public class AddDeclarationNodeToClassChange { private final ICPPASTCompositeTypeSpecifier classNode; private final VisibilityEnum visibility; - private List fieldNodes = new ArrayList(); + private final List nodesToAdd; private final ModificationCollector collector; - public static void createChange(ICPPASTCompositeTypeSpecifier nodeClass, - VisibilityEnum visibility, IASTNode fieldNodes, boolean isField, + public static void createChange(ICPPASTCompositeTypeSpecifier classNode, + VisibilityEnum visibility, IASTNode nodeToAdd, boolean isField, ModificationCollector collector) { - new AddDeclarationNodeToClassChange(nodeClass, visibility, fieldNodes, collector, isField); + createChange(classNode, visibility, Collections.singletonList(nodeToAdd), isField, collector); } public static void createChange(ICPPASTCompositeTypeSpecifier classNode, - VisibilityEnum visibility, List fieldNodes, boolean isField, + VisibilityEnum visibility, List nodesToAdd, boolean isField, ModificationCollector collector) { - new AddDeclarationNodeToClassChange(classNode, visibility, fieldNodes, collector, isField); + new AddDeclarationNodeToClassChange(classNode, visibility, nodesToAdd, collector, isField); } private AddDeclarationNodeToClassChange(ICPPASTCompositeTypeSpecifier classNode, - VisibilityEnum visibility, List fieldNodes, - ModificationCollector collector, boolean isField) { - this.fieldNodes = fieldNodes; - this.classNode = classNode; - this.visibility = visibility; - this.collector = collector; - createRewrites(isField); - } - - private AddDeclarationNodeToClassChange(ICPPASTCompositeTypeSpecifier classNode, - VisibilityEnum visibility, IASTNode fieldNodes, ModificationCollector collector, + VisibilityEnum visibility, List nodesToAdd, ModificationCollector collector, boolean isField) { + this.nodesToAdd = nodesToAdd; this.classNode = classNode; this.visibility = visibility; - this.fieldNodes.add(fieldNodes); this.collector = collector; createRewrites(isField); } @@ -137,7 +127,7 @@ public class AddDeclarationNodeToClassChange { private void insertBefore(IASTNode nextNode) { ASTRewrite rewrite = collector.rewriterForTranslationUnit(nextNode.getTranslationUnit()); - for (IASTNode node : fieldNodes) { + for (IASTNode node : nodesToAdd) { rewrite.insertBefore(nextNode.getParent(), nextNode, node, createEditDescription()); } } @@ -151,7 +141,7 @@ public class AddDeclarationNodeToClassChange { rewrite.insertBefore(classNode, null, label, createEditDescription()); } - for (IASTNode node : fieldNodes) { + for (IASTNode node : nodesToAdd) { rewrite.insertBefore(classNode, null, node, createEditDescription()); } } From 76e962ee9b55f4f365d3146f8cd63d8607e82963 Mon Sep 17 00:00:00 2001 From: Sergey Prigogin Date: Thu, 5 Jan 2012 20:21:29 -0800 Subject: [PATCH 3/5] @Override annotations. --- .../cpp/ICPPASTCompositeTypeSpecifier.java | 13 ++-- .../dom/ast/cpp/ICPPASTVisibilityLabel.java | 13 ++-- .../parser/c/CASTCompositeTypeSpecifier.java | 72 +++++++++++-------- .../cpp/CPPASTCompositeTypeSpecifier.java | 68 +++++++++++------- 4 files changed, 97 insertions(+), 69 deletions(-) diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/cpp/ICPPASTCompositeTypeSpecifier.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/cpp/ICPPASTCompositeTypeSpecifier.java index 96ab919ceca..edc38b6d605 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/cpp/ICPPASTCompositeTypeSpecifier.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/cpp/ICPPASTCompositeTypeSpecifier.java @@ -6,8 +6,8 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * John Camelon (IBM) - Initial API and implementation - * Markus Schorn (Wind River Systems) + * John Camelon (IBM) - Initial API and implementation + * Markus Schorn (Wind River Systems) *******************************************************************************/ package org.eclipse.cdt.core.dom.ast.cpp; @@ -22,10 +22,8 @@ import org.eclipse.cdt.core.dom.ast.IASTNode; * @noimplement This interface is not intended to be implemented by clients. */ public interface ICPPASTCompositeTypeSpecifier extends IASTCompositeTypeSpecifier, ICPPASTDeclSpecifier { - /** - * k_class C++ introduces the class concept for composite - * types. + * k_class C++ introduces the class concept for composite types. */ public static final int k_class = IASTCompositeTypeSpecifier.k_last + 1; @@ -52,15 +50,14 @@ public interface ICPPASTCompositeTypeSpecifier extends IASTCompositeTypeSpecifie * * @noimplement This interface is not intended to be implemented by clients. */ - public static interface ICPPASTBaseSpecifier extends IASTNode, IASTNameOwner, ICPPASTPackExpandable { - public static final ICPPASTBaseSpecifier[] EMPTY_BASESPECIFIER_ARRAY = new ICPPASTBaseSpecifier[0]; + public static interface ICPPASTBaseSpecifier extends IASTNode, IASTNameOwner, ICPPASTPackExpandable { + public static final ICPPASTBaseSpecifier[] EMPTY_BASESPECIFIER_ARRAY = {}; /** * Relation between base specifier and its name. */ public static final ASTNodeProperty NAME = new ASTNodeProperty( "ICPPASTBaseSpecifier.NAME - Name of base class"); //$NON-NLS-1$ - public static final int v_public = 1; public static final int v_protected = 2; diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/cpp/ICPPASTVisibilityLabel.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/cpp/ICPPASTVisibilityLabel.java index 6e4ece7cb26..23ba2025a7c 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/cpp/ICPPASTVisibilityLabel.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/cpp/ICPPASTVisibilityLabel.java @@ -6,7 +6,7 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * John Camelon (IBM) - Initial API and implementation + * John Camelon (IBM) - Initial API and implementation *******************************************************************************/ package org.eclipse.cdt.core.dom.ast.cpp; @@ -20,7 +20,6 @@ import org.eclipse.cdt.core.dom.ast.IASTDeclaration; * @noimplement This interface is not intended to be implemented by clients. */ public interface ICPPASTVisibilityLabel extends IASTDeclaration { - /** * v_public == public: */ @@ -37,28 +36,28 @@ public interface ICPPASTVisibilityLabel extends IASTDeclaration { public static final int v_private = 3; /** - * Get the visibility. + * Returns the visibility. * * @return int */ public int getVisibility(); /** - * Set visibility. + * Sets visibility. * - * @param visibility - * int + * @param visibility one of v_public, v_protected, v_private */ public void setVisibility(int visibility); - /** * @since 5.1 */ + @Override public ICPPASTVisibilityLabel copy(); /** * @since 5.3 */ + @Override public ICPPASTVisibilityLabel copy(CopyStyle style); } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/c/CASTCompositeTypeSpecifier.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/c/CASTCompositeTypeSpecifier.java index 158a3edfe78..dba54022524 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/c/CASTCompositeTypeSpecifier.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/c/CASTCompositeTypeSpecifier.java @@ -6,9 +6,9 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * John Camelon (IBM Rational Software) - Initial API and implementation - * Markus Schorn (Wind River Systems) - * Yuan Zhang / Beth Tibbitts (IBM Research) + * John Camelon (IBM Rational Software) - Initial API and implementation + * Markus Schorn (Wind River Systems) + * Yuan Zhang / Beth Tibbitts (IBM Research) *******************************************************************************/ package org.eclipse.cdt.internal.core.dom.parser.c; @@ -27,13 +27,12 @@ import org.eclipse.cdt.internal.core.dom.parser.IASTAmbiguityParent; */ public class CASTCompositeTypeSpecifier extends CASTBaseDeclSpecifier implements ICASTCompositeTypeSpecifier, IASTAmbiguityParent { - private int fKey; private IASTName fName; - private IASTDeclaration[] fActiveDeclarations= null; - private IASTDeclaration [] fAllDeclarations = null; - private int fDeclarationsPos=-1; - private IScope fScope = null; + private IASTDeclaration[] fActiveDeclarations; + private IASTDeclaration[] fAllDeclarations; + private int fDeclarationsPos = -1; + private IScope fScope; public CASTCompositeTypeSpecifier() { } @@ -43,10 +42,12 @@ public class CASTCompositeTypeSpecifier extends CASTBaseDeclSpecifier implements setName(name); } + @Override public CASTCompositeTypeSpecifier copy() { return copy(CopyStyle.withoutLocations); } + @Override public CASTCompositeTypeSpecifier copy(CopyStyle style) { CASTCompositeTypeSpecifier copy = new CASTCompositeTypeSpecifier(); copyCompositeTypeSpecifier(copy, style); @@ -60,24 +61,28 @@ public class CASTCompositeTypeSpecifier extends CASTBaseDeclSpecifier implements copyBaseDeclSpec(copy); copy.setKey(fKey); copy.setName(fName == null ? null : fName.copy(style)); - for(IASTDeclaration member : getMembers()) + for (IASTDeclaration member : getMembers()) copy.addMemberDeclaration(member == null ? null : member.copy(style)); } - public int getKey() { + @Override + public int getKey() { return fKey; } - public void setKey(int key) { + @Override + public void setKey(int key) { assertNotFrozen(); this.fKey = key; } - public IASTName getName() { + @Override + public IASTName getName() { return fName; } - public void setName(IASTName name) { + @Override + public void setName(IASTName name) { assertNotFrozen(); this.fName = name; if (name != null) { @@ -86,51 +91,57 @@ public class CASTCompositeTypeSpecifier extends CASTBaseDeclSpecifier implements } } + @Override public IASTDeclaration[] getMembers() { IASTDeclaration[] active= fActiveDeclarations; if (active == null) { - active = ASTQueries.extractActiveDeclarations(fAllDeclarations, fDeclarationsPos+1); + active = ASTQueries.extractActiveDeclarations(fAllDeclarations, fDeclarationsPos + 1); fActiveDeclarations= active; } return active; } + @Override public final IASTDeclaration[] getDeclarations(boolean includeInactive) { if (includeInactive) { - fAllDeclarations= (IASTDeclaration[]) ArrayUtil.removeNullsAfter(IASTDeclaration.class, fAllDeclarations, fDeclarationsPos); + fAllDeclarations= ArrayUtil.trimAt(IASTDeclaration.class, fAllDeclarations, + fDeclarationsPos); return fAllDeclarations; } return getMembers(); } - public void addMemberDeclaration(IASTDeclaration declaration) { + @Override + public void addMemberDeclaration(IASTDeclaration declaration) { assertNotFrozen(); if (declaration != null) { declaration.setParent(this); declaration.setPropertyInParent(MEMBER_DECLARATION); - fAllDeclarations = (IASTDeclaration[]) ArrayUtil.append(IASTDeclaration.class, fAllDeclarations, + fAllDeclarations = ArrayUtil.appendAt(IASTDeclaration.class, fAllDeclarations, ++fDeclarationsPos, declaration); fActiveDeclarations= null; } } - public void addDeclaration(IASTDeclaration declaration) { + @Override + public void addDeclaration(IASTDeclaration declaration) { addMemberDeclaration(declaration); } - - public IScope getScope() { - if( fScope == null ) - fScope = new CCompositeTypeScope( this ); + + @Override + public IScope getScope() { + if (fScope == null) + fScope = new CCompositeTypeScope(this); return fScope; } @Override - public boolean accept( ASTVisitor action ){ + public boolean accept(ASTVisitor action){ if (action.shouldVisitDeclSpecifiers) { switch (action.visit(this)) { - case ASTVisitor.PROCESS_ABORT : return false; - case ASTVisitor.PROCESS_SKIP : return true; - default : break; + case ASTVisitor.PROCESS_ABORT: return false; + case ASTVisitor.PROCESS_SKIP: return true; + default: break; } } if (fName != null && !fName.accept(action)) @@ -138,7 +149,8 @@ public class CASTCompositeTypeSpecifier extends CASTBaseDeclSpecifier implements IASTDeclaration[] decls= getDeclarations(action.includeInactiveNodes); for (int i = 0; i < decls.length; i++) { - if (!decls[i].accept(action)) return false; + if (!decls[i].accept(action)) + return false; } if (action.shouldVisitDeclSpecifiers) { @@ -151,13 +163,15 @@ public class CASTCompositeTypeSpecifier extends CASTBaseDeclSpecifier implements return true; } + @Override public int getRoleForName(IASTName n) { - if( n == this.fName ) + if (n == this.fName) return r_definition; return r_unclear; } - public void replace(IASTNode child, IASTNode other) { + @Override + public void replace(IASTNode child, IASTNode other) { assert child.isActive() == other.isActive(); for (int i = 0; i <= fDeclarationsPos; ++i) { if (fAllDeclarations[i] == child) { diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTCompositeTypeSpecifier.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTCompositeTypeSpecifier.java index c7752367055..20ef93b2382 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTCompositeTypeSpecifier.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTCompositeTypeSpecifier.java @@ -6,8 +6,8 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * John Camelon (IBM) - Initial API and implementation - * Markus Schorn (Wind River Systems) + * John Camelon (IBM) - Initial API and implementation + * Markus Schorn (Wind River Systems) *******************************************************************************/ package org.eclipse.cdt.internal.core.dom.parser.cpp; @@ -27,17 +27,15 @@ import org.eclipse.cdt.internal.core.dom.parser.IASTAmbiguityParent; */ public class CPPASTCompositeTypeSpecifier extends CPPASTBaseDeclSpecifier implements ICPPASTCompositeTypeSpecifier, IASTAmbiguityParent { - private int fKey; private IASTName fName; private CPPClassScope fScope; private IASTDeclaration[] fAllDeclarations; private IASTDeclaration[] fActiveDeclarations; - private int fDeclarationsPos=-1; - private ICPPASTCompositeTypeSpecifier.ICPPASTBaseSpecifier[] baseSpecs = null; + private int fDeclarationsPos = -1; + private ICPPASTCompositeTypeSpecifier.ICPPASTBaseSpecifier[] baseSpecs; private int baseSpecsPos = -1; - private boolean fAmbiguitiesResolved= false; - + private boolean fAmbiguitiesResolved; public CPPASTCompositeTypeSpecifier() { } @@ -54,13 +52,15 @@ public class CPPASTCompositeTypeSpecifier extends CPPASTBaseDeclSpecifier fAmbiguitiesResolved= true; } + @Override public CPPASTCompositeTypeSpecifier copy() { return copy(CopyStyle.withoutLocations); } + @Override public CPPASTCompositeTypeSpecifier copy(CopyStyle style) { - CPPASTCompositeTypeSpecifier copy = new CPPASTCompositeTypeSpecifier(fKey, fName == null - ? null : fName.copy(style)); + CPPASTCompositeTypeSpecifier copy = + new CPPASTCompositeTypeSpecifier(fKey, fName == null ? null : fName.copy(style)); copyBaseDeclSpec(copy); for (IASTDeclaration member : getMembers()) copy.addMemberDeclaration(member == null ? null : member.copy(style)); @@ -73,35 +73,43 @@ public class CPPASTCompositeTypeSpecifier extends CPPASTBaseDeclSpecifier return copy; } - public ICPPASTBaseSpecifier[] getBaseSpecifiers() { - if( baseSpecs == null ) return ICPPASTBaseSpecifier.EMPTY_BASESPECIFIER_ARRAY; - baseSpecs = (ICPPASTBaseSpecifier[]) ArrayUtil.removeNullsAfter( ICPPASTBaseSpecifier.class, baseSpecs, baseSpecsPos ); + @Override + public ICPPASTBaseSpecifier[] getBaseSpecifiers() { + if (baseSpecs == null) + return ICPPASTBaseSpecifier.EMPTY_BASESPECIFIER_ARRAY; + baseSpecs = ArrayUtil.trimAt(ICPPASTBaseSpecifier.class, baseSpecs, baseSpecsPos); return baseSpecs; } - public void addBaseSpecifier(ICPPASTBaseSpecifier baseSpec) { + @Override + public void addBaseSpecifier(ICPPASTBaseSpecifier baseSpec) { assertNotFrozen(); if (baseSpec != null) { baseSpec.setParent(this); baseSpec.setPropertyInParent(BASE_SPECIFIER); - baseSpecs = (ICPPASTBaseSpecifier[]) ArrayUtil.append( ICPPASTBaseSpecifier.class, baseSpecs, ++baseSpecsPos, baseSpec ); + baseSpecs = ArrayUtil.appendAt(ICPPASTBaseSpecifier.class, baseSpecs, ++baseSpecsPos, + baseSpec); } } - public int getKey() { + @Override + public int getKey() { return fKey; } - public void setKey(int key) { + @Override + public void setKey(int key) { assertNotFrozen(); fKey = key; } - public IASTName getName() { + @Override + public IASTName getName() { return fName; } - public void setName(IASTName name) { + @Override + public void setName(IASTName name) { assertNotFrozen(); this.fName = name; if (name != null) { @@ -110,23 +118,27 @@ public class CPPASTCompositeTypeSpecifier extends CPPASTBaseDeclSpecifier } } + @Override public IASTDeclaration[] getMembers() { IASTDeclaration[] active= fActiveDeclarations; if (active == null) { - active = ASTQueries.extractActiveDeclarations(fAllDeclarations, fDeclarationsPos+1); + active = ASTQueries.extractActiveDeclarations(fAllDeclarations, fDeclarationsPos + 1); fActiveDeclarations= active; } return active; } + @Override public final IASTDeclaration[] getDeclarations(boolean includeInactive) { if (includeInactive) { - fAllDeclarations= (IASTDeclaration[]) ArrayUtil.removeNullsAfter(IASTDeclaration.class, fAllDeclarations, fDeclarationsPos); + fAllDeclarations= ArrayUtil.trimAt(IASTDeclaration.class, fAllDeclarations, + fDeclarationsPos); return fAllDeclarations; } return getMembers(); } + @Override public void addMemberDeclaration(IASTDeclaration decl) { if (decl == null) return; @@ -138,15 +150,17 @@ public class CPPASTCompositeTypeSpecifier extends CPPASTBaseDeclSpecifier assertNotFrozen(); decl.setParent(this); decl.setPropertyInParent(decl instanceof ICPPASTVisibilityLabel ? VISIBILITY_LABEL : MEMBER_DECLARATION); - fAllDeclarations = (IASTDeclaration[]) ArrayUtil.append(IASTDeclaration.class, fAllDeclarations, + fAllDeclarations = ArrayUtil.appendAt(IASTDeclaration.class, fAllDeclarations, ++fDeclarationsPos, decl); fActiveDeclarations= null; } - public final void addDeclaration(IASTDeclaration decl) { + @Override + public final void addDeclaration(IASTDeclaration decl) { addMemberDeclaration(decl); } + @Override public ICPPClassScope getScope() { if (fScope == null) { fScope = new CPPClassScope(this); @@ -172,12 +186,14 @@ public class CPPASTCompositeTypeSpecifier extends CPPASTBaseDeclSpecifier ICPPASTBaseSpecifier[] bases = getBaseSpecifiers(); for (int i = 0; i < bases.length; i++) { - if (!bases[i].accept(action)) return false; + if (!bases[i].accept(action)) + return false; } IASTDeclaration[] decls = getDeclarations(action.includeInactiveNodes); for (int i = 0; i < decls.length; i++) { - if (!decls[i].accept(action)) return false; + if (!decls[i].accept(action)) + return false; } if (action.shouldVisitDeclSpecifiers && action.leave(this) == ASTVisitor.PROCESS_ABORT) @@ -186,12 +202,14 @@ public class CPPASTCompositeTypeSpecifier extends CPPASTBaseDeclSpecifier return true; } + @Override public int getRoleForName(IASTName name) { - if( name == this.fName ) + if (name == this.fName) return r_definition; return r_unclear; } + @Override public void replace(IASTNode child, IASTNode other) { assert child.isActive() == other.isActive(); for (int i = 0; i <= fDeclarationsPos; ++i) { From 046c25fadbb7af066116aa07b03cb1e497148742 Mon Sep 17 00:00:00 2001 From: Sergey Prigogin Date: Thu, 5 Jan 2012 20:23:58 -0800 Subject: [PATCH 4/5] Visibility ordering within a class declaration. --- .../resources/refactoring/ExtractConstant.rts | 17 +-- .../resources/refactoring/ExtractMethod.rts | 10 +- .../refactoring/GenerateGettersAndSetters.rts | 140 +++++++++--------- .../GenerateGettersAndSettersTest.java | 36 +++-- .../AddDeclarationNodeToClassChange.java | 120 +++++++-------- .../ui/refactoring/utils/VisibilityEnum.java | 52 +++---- .../eclipse/cdt/ui/PreferenceConstants.java | 80 +++++++--- 7 files changed, 245 insertions(+), 210 deletions(-) diff --git a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractConstant.rts b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractConstant.rts index a402270781e..8099b078d57 100644 --- a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractConstant.rts +++ b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractConstant.rts @@ -397,7 +397,6 @@ public: }; #endif /*A_H_*/ - //= #ifndef A_H_ #define A_H_ @@ -413,7 +412,6 @@ private: }; #endif /*A_H_*/ - //@A.cpp #include "A.h" @@ -426,7 +424,6 @@ A::~A() { int A::foo() { return /*$*/42/*$$*/; } - //= #include "A.h" @@ -439,8 +436,7 @@ A::~A() { int A::foo() { return theAnswer; } - -//!Bug 246062 [Refactoring] NPE extracting a constant from an inlined method +//!Bug 246062 - Extracting a constant from an inlined method //#org.eclipse.cdt.ui.tests.refactoring.extractconstant.ExtractConstantRefactoringTest //@.config filename=A.h @@ -450,18 +446,17 @@ class X { int a = /*$*/42/*$$*/; } }; - //= class X { +public: + static const int theAnswer = 42; + +private: void method() { int a = theAnswer; } - -public: - static const int theAnswer = 42; }; - -//!ExtractConstantString +//!Extract constant string //#org.eclipse.cdt.ui.tests.refactoring.extractconstant.ExtractConstantRefactoringTest //@.config visibility=private diff --git a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethod.rts b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethod.rts index 513d9d6d58d..368414c0540 100644 --- a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethod.rts +++ b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ExtractMethod.rts @@ -1480,7 +1480,6 @@ private: }; #endif /*A_H_*/ - //= #ifndef A_H_ #define A_H_ @@ -1491,15 +1490,14 @@ public: virtual ~A(); int foo(); -private: - int help(); - protected: void exp(int& i); + +private: + int help(); }; #endif /*A_H_*/ - //@A.cpp #include "A.h" @@ -1519,7 +1517,6 @@ int A::foo() { int A::help() { return 42; } - //= #include "A.h" @@ -1543,7 +1540,6 @@ int A::foo() { int A::help() { return 42; } - //@.config filename=A.cpp methodname=exp diff --git a/core/org.eclipse.cdt.ui.tests/resources/refactoring/GenerateGettersAndSetters.rts b/core/org.eclipse.cdt.ui.tests/resources/refactoring/GenerateGettersAndSetters.rts index 9d55f345d59..a270920614c 100644 --- a/core/org.eclipse.cdt.ui.tests/resources/refactoring/GenerateGettersAndSetters.rts +++ b/core/org.eclipse.cdt.ui.tests/resources/refactoring/GenerateGettersAndSetters.rts @@ -71,10 +71,6 @@ public: int GetUniqueId(); - char* getName() const { - return name; - } - int getSystemId() { return systemId; } @@ -82,6 +78,10 @@ public: void setSystemId(int systemId) { this->systemId = systemId; } + + char* getName() const { + return name; + } }; int gooo = 1; @@ -176,10 +176,6 @@ public: int GetUniqueId(); - char* getName() const { - return name; - } - int getSystemId() { return systemId; } @@ -187,6 +183,10 @@ public: void setSystemId(int systemId) { this->systemId = systemId; } + + char* getName() const { + return name; + } }; } @@ -246,7 +246,6 @@ public: } }; - int gooo = 1; #endif /* A_H_ */ @@ -278,10 +277,6 @@ public: int GetUniqueId(); - void setName(char* name) { - this->name = name; - } - int getSystemId() { return systemId; } @@ -289,8 +284,11 @@ public: void setSystemId(int systemId) { this->systemId = systemId; } -}; + void setName(char* name) { + this->name = name; + } +}; int gooo = 1; @@ -379,14 +377,6 @@ public: int GetUniqueId(); - char* getName() const { - return name; - } - - void setName(char* name) { - this->name = name; - } - int getSystemId() { return systemId; } @@ -394,6 +384,14 @@ public: void setSystemId(int systemId) { this->systemId = systemId; } + + char* getName() const { + return name; + } + + void setName(char* name) { + this->name = name; + } }; int gooo = 1; @@ -463,6 +461,18 @@ public: Person myFriend; + Person(int socSecNo); // constructor + + ~Person(); // destructor + + char* Name(); + + void Print(); + + int SocSecNo(); + + int GetUniqueId(); + char* getName() const { return name; } @@ -478,18 +488,6 @@ public: void setSystemId(int systemId) { this->systemId = systemId; } - - Person(int socSecNo); // constructor - - ~Person(); // destructor - - char* Name(); - - void Print(); - - int SocSecNo(); - - int GetUniqueId(); }; int gooo = 1; @@ -567,7 +565,7 @@ GaS::Getters() { GaS::~Getters() { } -//!Generate Getters and Setters no Methods +//!Generate Getters and Setters no methods //#org.eclipse.cdt.ui.tests.refactoring.gettersandsetters.GenerateGettersAndSettersTest //@.config filename=A.h @@ -588,9 +586,6 @@ private: #define A_H_ class Person { -private: - int id; - public: int getId() const { return id; @@ -599,15 +594,19 @@ public: void setId(int id) { this->id = id; } + +private: + int id; }; #endif /* A_H_ */ -//!Generate Getters and Setters no Methods +//!Generate Getters and Setters no methods ascending visibility order //#org.eclipse.cdt.ui.tests.refactoring.gettersandsetters.GenerateGettersAndSettersTest //@.config filename=A.h getters=i setters=i +ascendingVisibilityOrder=true //@A.h /* * test.h @@ -624,7 +623,6 @@ class test { }; #endif /* TEST_H_ */ - //= /* * test.h @@ -650,8 +648,7 @@ public: }; #endif /* TEST_H_ */ - -//!Generate Getters and Setters no Fields +//!Generate Getters and Setters no fields //#org.eclipse.cdt.ui.tests.refactoring.gettersandsetters.GenerateGettersAndSettersTest //@.config filename=A.h @@ -759,8 +756,6 @@ public: Person myFriend; - const FullName& getName() const; - void setName(const FullName& name); Person(int socSecNo); // constructor ~Person(); // destructor @@ -776,6 +771,8 @@ public: int getSystemId(); void setSystemId(int systemId); + const FullName& getName() const; + void setName(const FullName& name); }; int gooo = 1; @@ -886,7 +883,6 @@ public: int SocSecNo(); int GetUniqueId(); - char* getName() const; int getSystemId() { return systemId; @@ -895,6 +891,8 @@ public: void setSystemId(int systemId) { this->systemId = systemId; } + + char* getName() const; }; } @@ -1005,7 +1003,6 @@ public: int SocSecNo(); int GetUniqueId(); - void setName(char* name); int getSystemId() { return systemId; @@ -1014,6 +1011,8 @@ public: void setSystemId(int systemId) { this->systemId = systemId; } + + void setName(char* name); }; int gooo = 1; @@ -1115,8 +1114,6 @@ public: int SocSecNo(); int GetUniqueId(); - char* getName() const; - void setName(char* name); int getSystemId() { return systemId; @@ -1125,6 +1122,9 @@ public: void setSystemId(int systemId) { this->systemId = systemId; } + + char* getName() const; + void setName(char* name); }; int gooo = 1; @@ -1178,12 +1178,12 @@ private: #define A_H_ class Person { -private: - int id; - public: int getId() const; void setId(int id); + +private: + int id; }; inline int Person::getId() const { @@ -1228,13 +1228,14 @@ class test { //comment1 class test { - int i; //comment2 - char* b; - //comment3 - public: int getI() const; void setI(int i); + +private: + int i; //comment2 + char* b; + //comment3 }; inline int test::getI() const { @@ -1275,12 +1276,13 @@ class Test { namespace foo { class Test { - int testField; - void foo(); - public: int getTestField() const; void setTestField(int testField); + +private: + int testField; + void foo(); }; } // namespace foo @@ -1334,11 +1336,12 @@ class Test { #define TEST_H_ class Test { - int testField; - public: int getTestField() const; void setTestField(int testField); + +private: + int testField; }; #endif @@ -1374,11 +1377,12 @@ class Test { #define TEST_H_ class Test { - int testField; - public: int getTestField() const; void setTestField(int testField); + +private: + int testField; }; #endif @@ -1464,13 +1468,13 @@ private: #define A_H_ class A { -private: - int a[2]; - public: const int* getA() const { return a; } + +private: + int a[2]; }; #endif /* A_H_ */ //!Bug 352258 - Avoiding reserved names @@ -1493,9 +1497,6 @@ private: #define A_H_ class getClass { -private: - int mClass; - public: int getClass1() const { return mClass; @@ -1504,5 +1505,8 @@ public: void setClass(int _class) { mClass = _class; } + +private: + int mClass; }; #endif /* A_H_ */ diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/gettersandsetters/GenerateGettersAndSettersTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/gettersandsetters/GenerateGettersAndSettersTest.java index 6b9cbf91d44..2cdeca22683 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/gettersandsetters/GenerateGettersAndSettersTest.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/gettersandsetters/GenerateGettersAndSettersTest.java @@ -18,11 +18,14 @@ import java.util.Properties; import org.eclipse.core.resources.IFile; import org.eclipse.core.runtime.CoreException; +import org.eclipse.jface.preference.IPreferenceStore; import org.eclipse.ltk.core.refactoring.Change; import org.eclipse.ltk.core.refactoring.RefactoringStatus; import org.eclipse.cdt.core.model.CoreModel; import org.eclipse.cdt.core.model.ICElement; +import org.eclipse.cdt.ui.CUIPlugin; +import org.eclipse.cdt.ui.PreferenceConstants; import org.eclipse.cdt.ui.tests.refactoring.RefactoringTest; import org.eclipse.cdt.ui.tests.refactoring.TestSourceFile; @@ -41,6 +44,7 @@ public class GenerateGettersAndSettersTest extends RefactoringTest { private String[] selectedSetters; private GenerateGettersAndSettersRefactoring refactoring; private boolean definitionSeparate; + private String ascendingVisibilityOrder; /** * @param name @@ -52,21 +56,30 @@ public class GenerateGettersAndSettersTest extends RefactoringTest { @Override protected void runTest() throws Throwable { - IFile file = project.getFile(fileName); - ICElement element = CoreModel.getDefault().create(file); - refactoring = new GenerateGettersAndSettersRefactoring(element, selection, cproject, astCache); - RefactoringStatus initialConditions = refactoring.checkInitialConditions(NULL_PROGRESS_MONITOR); - - if (fatalError) { - assertConditionsFatalError(initialConditions); - return; - } else { - assertConditionsOk(initialConditions); - executeRefactoring(); + try { + IFile file = project.getFile(fileName); + ICElement element = CoreModel.getDefault().create(file); + refactoring = new GenerateGettersAndSettersRefactoring(element, selection, cproject, astCache); + RefactoringStatus initialConditions = refactoring.checkInitialConditions(NULL_PROGRESS_MONITOR); + + if (fatalError) { + assertConditionsFatalError(initialConditions); + return; + } else { + assertConditionsOk(initialConditions); + executeRefactoring(); + } + } finally { + IPreferenceStore store= CUIPlugin.getDefault().getPreferenceStore(); + store.setToDefault(PreferenceConstants.CLASS_MEMBER_ASCENDING_VISIBILITY_ORDER); } } private void executeRefactoring() throws CoreException, Exception { + if (ascendingVisibilityOrder != null) { + IPreferenceStore store= CUIPlugin.getDefault().getPreferenceStore(); + store.setValue(PreferenceConstants.CLASS_MEMBER_ASCENDING_VISIBILITY_ORDER, ascendingVisibilityOrder); + } selectFields(); refactoring.getContext().setDefinitionSeparate(definitionSeparate); RefactoringStatus finalConditions = refactoring.checkFinalConditions(NULL_PROGRESS_MONITOR); @@ -102,6 +115,7 @@ public class GenerateGettersAndSettersTest extends RefactoringTest { String getters = refactoringProperties.getProperty("getters", ""); //$NON-NLS-1$ //$NON-NLS-2$ String setters = refactoringProperties.getProperty("setters", ""); //$NON-NLS-1$ //$NON-NLS-2$ definitionSeparate = Boolean.valueOf(refactoringProperties.getProperty("definitionSeparate", "false")); + ascendingVisibilityOrder = refactoringProperties.getProperty("ascendingVisibilityOrder", null); selectedGetters = getters.split(","); selectedSetters = setters.split(","); diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/AddDeclarationNodeToClassChange.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/AddDeclarationNodeToClassChange.java index 2cd2b770f05..f0c09880283 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/AddDeclarationNodeToClassChange.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/AddDeclarationNodeToClassChange.java @@ -8,12 +8,17 @@ * * Contributors: * Institute for Software - initial API and implementation + * Sergey Prigogin (Google) *******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring; +import java.util.ArrayList; import java.util.Collections; import java.util.List; +import org.eclipse.core.resources.IProject; +import org.eclipse.core.runtime.Platform; +import org.eclipse.core.runtime.preferences.IPreferencesService; import org.eclipse.osgi.util.NLS; import org.eclipse.text.edits.TextEditGroup; @@ -23,9 +28,14 @@ import org.eclipse.cdt.core.dom.ast.IASTDeclarator; import org.eclipse.cdt.core.dom.ast.IASTFunctionDeclarator; import org.eclipse.cdt.core.dom.ast.IASTNode; import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration; +import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTCompositeTypeSpecifier; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTFunctionDefinition; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTVisibilityLabel; import org.eclipse.cdt.core.dom.rewrite.ASTRewrite; +import org.eclipse.cdt.core.model.ITranslationUnit; +import org.eclipse.cdt.ui.CUIPlugin; +import org.eclipse.cdt.ui.PreferenceConstants; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTVisibilityLabel; @@ -58,7 +68,7 @@ public class AddDeclarationNodeToClassChange { private AddDeclarationNodeToClassChange(ICPPASTCompositeTypeSpecifier classNode, VisibilityEnum visibility, List nodesToAdd, ModificationCollector collector, boolean isField) { - this.nodesToAdd = nodesToAdd; + this.nodesToAdd = new ArrayList(nodesToAdd); this.classNode = classNode; this.visibility = visibility; this.collector = collector; @@ -66,83 +76,65 @@ public class AddDeclarationNodeToClassChange { } private void createRewrites(boolean isField) { - int lastFunctionDeclaration = -1; - int lastFieldDeclaration = -1; + VisibilityEnum defaultVisibility = classNode.getKey() == IASTCompositeTypeSpecifier.k_struct ? + VisibilityEnum.v_public : VisibilityEnum.v_private; + VisibilityEnum currentVisibility = defaultVisibility; + + boolean ascendingVisibilityOrder = isAscendingVisibilityOrder(); + int lastFunctionIndex = -1; + int lastFieldIndex = -1; + int lastMatchingVisibilityIndex = -1; + int lastPrecedingVisibilityIndex = -1; IASTDeclaration[] members = classNode.getMembers(); - VisibilityEnum currentVisibility = classNode.getKey() == IASTCompositeTypeSpecifier.k_struct ? - VisibilityEnum.v_public : VisibilityEnum.v_private; - // Find the insert location by iterating over the elements of the class - // and remembering the last element with the matching visibility + // and remembering the last element with the matching visibility and the last element + // with preceding visibility (according to the visibility order preference). for (int i = 0; i < members.length; i++) { IASTDeclaration declaration = members[i]; if (declaration instanceof ICPPASTVisibilityLabel) { currentVisibility = VisibilityEnum.from((ICPPASTVisibilityLabel) declaration); - } else if (declaration instanceof IASTSimpleDeclaration && currentVisibility.equals(visibility)) { - IASTSimpleDeclaration simple = (IASTSimpleDeclaration) declaration; - IASTDeclarator[] declarators = simple.getDeclarators(); - if (declarators.length > 0 && declarators[0] != null) { - if (declarators[0] instanceof IASTFunctionDeclarator) { - lastFunctionDeclaration = i; - } else { - lastFieldDeclaration = i; + } + if (currentVisibility == visibility) { + lastMatchingVisibilityIndex = i; + if (declaration instanceof IASTSimpleDeclaration) { + IASTDeclarator[] declarators = ((IASTSimpleDeclaration) declaration).getDeclarators(); + if (declarators.length > 0 && declarators[0] != null) { + if (declarators[0] instanceof IASTFunctionDeclarator) { + lastFunctionIndex = i; + } else { + lastFieldIndex = i; + } } + } else if (declaration instanceof ICPPASTFunctionDefinition) { + lastFunctionIndex = i; } + } else if (currentVisibility.compareTo(visibility) < 0 == ascendingVisibilityOrder) { + lastPrecedingVisibilityIndex = i; } } - IASTDeclaration nextFunctionDeclaration = null; - if (lastFunctionDeclaration < members.length - 1 && lastFunctionDeclaration >= 0) - nextFunctionDeclaration = members[lastFunctionDeclaration + 1]; - - IASTDeclaration nextFieldDeclaration = null; - if (lastFieldDeclaration < members.length - 1 && lastFieldDeclaration >= 0) - nextFieldDeclaration = members[lastFieldDeclaration + 1]; + int index = isField && lastFieldIndex >= 0 || !isField && lastFunctionIndex < 0 ? + lastFieldIndex : lastFunctionIndex; + if (index < 0) + index = lastMatchingVisibilityIndex; + if (index < 0) + index = lastPrecedingVisibilityIndex; + index++; + IASTNode nextNode = index < members.length ? members[index] : null; - createInsert(isField, nextFunctionDeclaration, nextFieldDeclaration, currentVisibility); - } - - private void createInsert(boolean isField, IASTDeclaration nextFunctionDeclaration, - IASTDeclaration nextFieldDeclaration, VisibilityEnum currentVisibility) { - if (isField) { - if (nextFieldDeclaration != null) { - insertBefore(nextFieldDeclaration); - } else if (nextFunctionDeclaration != null) { - insertBefore(nextFunctionDeclaration); - } else { - insertAtTheEnd(currentVisibility); - } - } else { - if (nextFunctionDeclaration != null) { - insertBefore(nextFunctionDeclaration); - } else if (nextFieldDeclaration != null) { - insertBefore(nextFieldDeclaration); - } else { - insertAtTheEnd(currentVisibility); + if (lastMatchingVisibilityIndex < 0 && + !(index == 0 && classNode.getKey() == IASTCompositeTypeSpecifier.k_struct && visibility == defaultVisibility)) { + nodesToAdd.add(0, new CPPASTVisibilityLabel(visibility.getVisibilityLabelValue())); + if (index == 0 && nextNode != null && !(nextNode instanceof ICPPASTVisibilityLabel)) { + nodesToAdd.add(new CPPASTVisibilityLabel(defaultVisibility.getVisibilityLabelValue())); } } - } - private void insertBefore(IASTNode nextNode) { - ASTRewrite rewrite = collector.rewriterForTranslationUnit(nextNode.getTranslationUnit()); - for (IASTNode node : nodesToAdd) { - rewrite.insertBefore(nextNode.getParent(), nextNode, node, createEditDescription()); - } - } - - private void insertAtTheEnd(VisibilityEnum currentVisibility) { ASTRewrite rewrite = collector.rewriterForTranslationUnit(classNode.getTranslationUnit()); - - if (!currentVisibility.equals(visibility)) { - ICPPASTVisibilityLabel label = - new CPPASTVisibilityLabel(visibility.getICPPASTVisiblityLabelVisibility()); - rewrite.insertBefore(classNode, null, label, createEditDescription()); - } - for (IASTNode node : nodesToAdd) { - rewrite.insertBefore(classNode, null, node, createEditDescription()); + rewrite.insertBefore(classNode, nextNode, node, createEditDescription()); } } @@ -150,4 +142,14 @@ public class AddDeclarationNodeToClassChange { return new TextEditGroup(NLS.bind(Messages.AddDeclarationNodeToClassChange_AddDeclaration, classNode.getName())); } + + private boolean isAscendingVisibilityOrder() { + IPreferencesService preferences = Platform.getPreferencesService(); + IASTTranslationUnit ast = classNode.getTranslationUnit(); + ITranslationUnit tu = ast.getOriginatingTranslationUnit(); + IProject project = tu != null ? tu.getCProject().getProject() : null; + return preferences.getBoolean(CUIPlugin.PLUGIN_ID, + PreferenceConstants.CLASS_MEMBER_ASCENDING_VISIBILITY_ORDER, false, + PreferenceConstants.getPreferenceScopes(project)); + } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/VisibilityEnum.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/VisibilityEnum.java index 2203263f61c..19f87e09e0f 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/VisibilityEnum.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/VisibilityEnum.java @@ -7,7 +7,7 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * Institute for Software - initial API and implementation + * Institute for Software - initial API and implementation *******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring.utils; @@ -17,22 +17,26 @@ import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTCompositeTypeSpecifier.ICPPASTBas /** * Enum that represents C++ visibilities, with methods to convert to * and from ICPPASTVisiblityLabel. - * */ public enum VisibilityEnum { - - v_public(Messages.VisibilityEnum_public), - v_protected(Messages.VisibilityEnum_protected), - v_private(Messages.VisibilityEnum_private); + // The values are ordered by increasing visibility. + v_private(Messages.VisibilityEnum_private, ICPPASTVisibilityLabel.v_private, ICPPASTBaseSpecifier.v_private), + v_protected(Messages.VisibilityEnum_protected, ICPPASTVisibilityLabel.v_protected, ICPPASTBaseSpecifier.v_protected), + v_public(Messages.VisibilityEnum_public, ICPPASTVisibilityLabel.v_public, ICPPASTBaseSpecifier.v_public); private final String stringRepresentation; + private final int visibilityLabelValue; + private final int baseSpecifierValue; - VisibilityEnum(String stringRepresentation) { + private VisibilityEnum(String stringRepresentation, int visibilityLabelValue, + int baseSpecifierValue) { this.stringRepresentation = stringRepresentation; + this.visibilityLabelValue = visibilityLabelValue; + this.baseSpecifierValue = baseSpecifierValue; } public static VisibilityEnum from(ICPPASTVisibilityLabel visibility) { - switch(visibility.getVisibility()){ + switch (visibility.getVisibility()) { case ICPPASTVisibilityLabel.v_private: return VisibilityEnum.v_private; case ICPPASTVisibilityLabel.v_protected: @@ -43,36 +47,20 @@ public enum VisibilityEnum { return null; } - public int getASTBaseSpecifierVisibility() { - switch (this) { - case v_private: - return ICPPASTBaseSpecifier.v_private; - case v_protected: - return ICPPASTBaseSpecifier.v_protected; - case v_public: - return ICPPASTBaseSpecifier.v_public; - } - return 0; + public int getBaseSpecifierValue() { + return baseSpecifierValue; } - public int getICPPASTVisiblityLabelVisibility() { - switch (this) { - case v_private: - return ICPPASTVisibilityLabel.v_private; - case v_protected: - return ICPPASTVisibilityLabel.v_protected; - case v_public: - return ICPPASTVisibilityLabel.v_public; - } - return 0; + public int getVisibilityLabelValue() { + return visibilityLabelValue; } - public static VisibilityEnum getEnumForStringRepresentation(String visibility){ - if( VisibilityEnum.v_private.toString().equals( visibility ) ) { + public static VisibilityEnum getEnumForStringRepresentation(String visibility) { + if (VisibilityEnum.v_private.toString().equals(visibility)) { return VisibilityEnum.v_private; - }else if( VisibilityEnum.v_protected.toString().equals( visibility ) ) { + } else if (VisibilityEnum.v_protected.toString().equals(visibility)) { return VisibilityEnum.v_protected; - }else if ( VisibilityEnum.v_public.toString().equals( visibility ) ) { + } else if (VisibilityEnum.v_public.toString().equals(visibility)) { return VisibilityEnum.v_public; } return null; diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/PreferenceConstants.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/PreferenceConstants.java index a979aa5cb8e..1e553a04244 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/PreferenceConstants.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/PreferenceConstants.java @@ -17,9 +17,12 @@ package org.eclipse.cdt.ui; import java.util.Locale; +import org.eclipse.core.resources.IProject; import org.eclipse.core.resources.ProjectScope; +import org.eclipse.core.runtime.preferences.ConfigurationScope; import org.eclipse.core.runtime.preferences.DefaultScope; import org.eclipse.core.runtime.preferences.IEclipsePreferences; +import org.eclipse.core.runtime.preferences.IScopeContext; import org.eclipse.core.runtime.preferences.InstanceScope; import org.eclipse.jface.action.Action; import org.eclipse.jface.preference.IPreferenceStore; @@ -597,7 +600,8 @@ public class PreferenceConstants { public final static String EDITOR_TEXT_FONT= "org.eclipse.cdt.ui.editors.textfont"; //$NON-NLS-1$ /** - * A named preference that controls whether the cview's selection is linked to the active editor. + * A named preference that controls whether the cview's selection is linked to the active + * editor. *

* Value is of type Boolean. *

@@ -605,7 +609,8 @@ public class PreferenceConstants { public static final String PREF_LINK_TO_EDITOR= "org.eclipse.cdt.ui.editor.linkToEditor"; //$NON-NLS-1$ /** - * A named preference that speficies whether children of a translation unit are shown in the CView. + * A named preference that specifies whether children of a translation unit are shown in + * the CView. *

* Value is of type Boolean. *

@@ -613,7 +618,8 @@ public class PreferenceConstants { public static final String PREF_SHOW_CU_CHILDREN= "org.eclipse.cdt.ui.editor.CUChildren"; //$NON-NLS-1$ /** - * A named preference that speficies whether to use the parser's structural mode to build the CModel. + * A named preference that speficies whether to use the parser's structural mode to build + * the CModel. *

* Value is of type Boolean. *

@@ -621,7 +627,8 @@ public class PreferenceConstants { public static final String PREF_USE_STRUCTURAL_PARSE_MODE= "org.eclipse.cdt.ui.editor.UseStructuralMode"; //$NON-NLS-1$ /** - * A named preference that controls if segmented view (show selected element only) is turned on or off. + * A named preference that controls if segmented view (show selected element only) is turned + * on or off. *

* Value is of type Boolean. *

@@ -818,8 +825,8 @@ public class PreferenceConstants { public static final String OUTLINE_LINK_TO_EDITOR = "org.eclipse.cdt.ui.outline.linktoeditor"; //$NON-NLS-1$ /** - * A named preference that controls whether include directives should be grouped in the - * C/C++ Projects view and the Project Explorer view. + * A named preference that controls whether include directives should be grouped in + * the C/C++ Projects view and the Project Explorer view. *

* Value is of type Boolean. *

@@ -827,8 +834,8 @@ public class PreferenceConstants { public static final String CVIEW_GROUP_INCLUDES= "org.eclipse.cdt.ui.cview.groupincludes"; //$NON-NLS-1$ /** - * A named preference that controls whether macro defintions should be grouped in the - * C/C++ Projects view and the Project Explorer view. + * A named preference that controls whether macro definitions should be grouped in + * the C/C++ Projects view and the Project Explorer view. *

* Value is of type Boolean. *

@@ -837,8 +844,8 @@ public class PreferenceConstants { public static final String CVIEW_GROUP_MACROS= "org.eclipse.cdt.ui.cview.groupmacros"; //$NON-NLS-1$ /** - * A named preference that controls whether header and source files should be separated in the - * C/C++ Projects view and the Project Explorer view. + * A named preference that controls whether header and source files should be separated in + * the C/C++ Projects view and the Project Explorer view. *

* Value is of type Boolean. *

@@ -1878,6 +1885,18 @@ public class PreferenceConstants { */ public static final int NAME_STYLE_CAPITALIZATION_LOWER_CAMEL_CASE = 4; + /** + * A named preference that controls order of private/protected/public class members in generated + * code. + *

+ * Value is of type Boolean. The true value means that private members + * are before public ones. The default is to put public members before private ones. + * + * @since 5.4 + */ + public static final String CLASS_MEMBER_ASCENDING_VISIBILITY_ORDER = "class_member_ascending_visibility_order"; //$NON-NLS-1$ + + /** * Returns the CDT-UI preference store. * @@ -2089,9 +2108,9 @@ public class PreferenceConstants { /** * Returns the node in the preference in the given context. * @param key The preference key. - * @param project The current context or null if no context is available and the - * workspace setting should be taken. Note that passing null should - * be avoided. + * @param project The current context or null if no context is available and + * the workspace setting should be taken. Note that passing null should + * be avoided. * @return Returns the node matching the given context. */ private static IEclipsePreferences getPreferenceNode(String key, ICProject project) { @@ -2108,15 +2127,20 @@ public class PreferenceConstants { return node; } + node = ConfigurationScope.INSTANCE.getNode(CUIPlugin.PLUGIN_ID); + if (node.get(key, null) != null) { + return node; + } + return DefaultScope.INSTANCE.getNode(CUIPlugin.PLUGIN_ID); } /** * Returns the string value for the given key in the given context. * @param key The preference key - * @param project The current context or null if no context is available and the - * workspace setting should be taken. Note that passing null should - * be avoided. + * @param project The current context or null if no context is available and + * the workspace setting should be taken. Note that passing null should + * be avoided. * @return Returns the current value for the string. * @since 5.0 */ @@ -2127,9 +2151,9 @@ public class PreferenceConstants { /** * Returns the integer value for the given key in the given context. * @param key The preference key - * @param project The current context or null if no context is available and the - * workspace setting should be taken. Note that passing null should - * be avoided. + * @param project The current context or null if no context is available and + * the workspace setting should be taken. Note that passing null should + * be avoided. * @param defaultValue The default value if not specified in the preferences. * @return Returns the current value for the string. * @since 5.1 @@ -2141,9 +2165,9 @@ public class PreferenceConstants { /** * Returns the boolean value for the given key in the given context. * @param key The preference key - * @param project The current context or null if no context is available and the - * workspace setting should be taken. Note that passing null should - * be avoided. + * @param project The current context or null if no context is available and + * the workspace setting should be taken. Note that passing null should + * be avoided. * @param defaultValue The default value if not specified in the preferences. * @return Returns the current value for the string. * @since 5.1 @@ -2152,4 +2176,16 @@ public class PreferenceConstants { return getPreferenceNode(key, project).getBoolean(key, defaultValue); } + /** + * Returns the scopes for preference lookup. + * + * @param project a project or null + * @return the scopes for preference lookup. + * @since 5.4 + */ + public static IScopeContext[] getPreferenceScopes(IProject project) { + return project != null ? + new IScopeContext[] { new ProjectScope(project), InstanceScope.INSTANCE, ConfigurationScope.INSTANCE, DefaultScope.INSTANCE } : + new IScopeContext[] { InstanceScope.INSTANCE, ConfigurationScope.INSTANCE, DefaultScope.INSTANCE }; + } } From be46e98db2a2f4658d9b3e5b2d304aa993f03e33 Mon Sep 17 00:00:00 2001 From: Sergey Prigogin Date: Thu, 5 Jan 2012 20:29:09 -0800 Subject: [PATCH 5/5] Renamed AddDeclarationNodeToClassChange to ClassMemberInserter. --- ...NodeToClassChange.java => ClassMemberInserter.java} | 10 +++++----- .../extractconstant/ExtractConstantRefactoring.java | 4 ++-- .../extractfunction/ExtractFunctionRefactoring.java | 4 ++-- .../GenerateGettersAndSettersRefactoring.java | 4 ++-- .../refactoring/hidemethod/HideMethodRefactoring.java | 4 ++-- 5 files changed, 13 insertions(+), 13 deletions(-) rename core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/{AddDeclarationNodeToClassChange.java => ClassMemberInserter.java} (95%) diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/AddDeclarationNodeToClassChange.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/ClassMemberInserter.java similarity index 95% rename from core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/AddDeclarationNodeToClassChange.java rename to core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/ClassMemberInserter.java index f0c09880283..04b7f420e38 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/AddDeclarationNodeToClassChange.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/ClassMemberInserter.java @@ -42,12 +42,12 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTVisibilityLabel; import org.eclipse.cdt.internal.ui.refactoring.utils.VisibilityEnum; /** - * Adds a declaration to an existing class via the ModificationCollector. It automatically searches - * the correct insertion point for the desired visibility. + * Adds a declaration to an existing class via the ModificationCollector. Automatically determines + * an appropriate insertion point for the desired visibility. * * @author Mirko Stocker */ -public class AddDeclarationNodeToClassChange { +public class ClassMemberInserter { private final ICPPASTCompositeTypeSpecifier classNode; private final VisibilityEnum visibility; private final List nodesToAdd; @@ -62,10 +62,10 @@ public class AddDeclarationNodeToClassChange { public static void createChange(ICPPASTCompositeTypeSpecifier classNode, VisibilityEnum visibility, List nodesToAdd, boolean isField, ModificationCollector collector) { - new AddDeclarationNodeToClassChange(classNode, visibility, nodesToAdd, collector, isField); + new ClassMemberInserter(classNode, visibility, nodesToAdd, collector, isField); } - private AddDeclarationNodeToClassChange(ICPPASTCompositeTypeSpecifier classNode, + private ClassMemberInserter(ICPPASTCompositeTypeSpecifier classNode, VisibilityEnum visibility, List nodesToAdd, ModificationCollector collector, boolean isField) { this.nodesToAdd = new ArrayList(nodesToAdd); diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoring.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoring.java index 3adaf1eba85..f3467889f49 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoring.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoring.java @@ -65,7 +65,7 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTName; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTSimpleDeclaration; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPMethod; -import org.eclipse.cdt.internal.ui.refactoring.AddDeclarationNodeToClassChange; +import org.eclipse.cdt.internal.ui.refactoring.ClassMemberInserter; import org.eclipse.cdt.internal.ui.refactoring.CRefactoring; import org.eclipse.cdt.internal.ui.refactoring.CRefactoringDescription; import org.eclipse.cdt.internal.ui.refactoring.MethodContext; @@ -325,7 +325,7 @@ public class ExtractConstantRefactoring extends CRefactoring { if (context.getType() == MethodContext.ContextType.METHOD) { ICPPASTCompositeTypeSpecifier classDefinition = (ICPPASTCompositeTypeSpecifier) context.getMethodDeclaration().getParent(); - AddDeclarationNodeToClassChange.createChange(classDefinition, info.getVisibility(), getConstNodesClass(constName), true, collector); + ClassMemberInserter.createChange(classDefinition, info.getVisibility(), getConstNodesClass(constName), true, collector); } else { IASTDeclaration nodes = getConstNodesGlobal(constName); ASTRewrite rewriter = collector.rewriterForTranslationUnit(ast); diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoring.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoring.java index e53a3e7f3f3..c2c85aaf906 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoring.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoring.java @@ -95,7 +95,7 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPNodeFactory; import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPVariableReadWriteFlags; import org.eclipse.cdt.internal.core.pdom.dom.PDOMName; -import org.eclipse.cdt.internal.ui.refactoring.AddDeclarationNodeToClassChange; +import org.eclipse.cdt.internal.ui.refactoring.ClassMemberInserter; import org.eclipse.cdt.internal.ui.refactoring.CRefactoring; import org.eclipse.cdt.internal.ui.refactoring.CRefactoringDescription; import org.eclipse.cdt.internal.ui.refactoring.Container; @@ -419,7 +419,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { IASTSimpleDeclaration methodDeclaration = getDeclaration(collector, astMethodName); - AddDeclarationNodeToClassChange.createChange(classDeclaration, info.getVisibility(), + ClassMemberInserter.createChange(classDeclaration, info.getVisibility(), methodDeclaration, false, collector); } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GenerateGettersAndSettersRefactoring.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GenerateGettersAndSettersRefactoring.java index f1f3e6794e5..813beba470f 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GenerateGettersAndSettersRefactoring.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GenerateGettersAndSettersRefactoring.java @@ -47,7 +47,7 @@ import org.eclipse.cdt.core.model.ICProject; import org.eclipse.cdt.internal.core.dom.rewrite.astwriter.ContainerNode; import org.eclipse.cdt.internal.ui.refactoring.CRefactoring2; -import org.eclipse.cdt.internal.ui.refactoring.AddDeclarationNodeToClassChange; +import org.eclipse.cdt.internal.ui.refactoring.ClassMemberInserter; import org.eclipse.cdt.internal.ui.refactoring.Container; import org.eclipse.cdt.internal.ui.refactoring.ModificationCollector; import org.eclipse.cdt.internal.ui.refactoring.RefactoringASTCache; @@ -249,7 +249,7 @@ public class GenerateGettersAndSettersRefactoring extends CRefactoring2 { ICPPASTCompositeTypeSpecifier classDefinition = (ICPPASTCompositeTypeSpecifier) context.existingFields.get(context.existingFields.size() - 1).getParent(); - AddDeclarationNodeToClassChange.createChange(classDefinition, VisibilityEnum.v_public, + ClassMemberInserter.createChange(classDefinition, VisibilityEnum.v_public, getterAndSetters, false, collector); } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/hidemethod/HideMethodRefactoring.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/hidemethod/HideMethodRefactoring.java index c9de2a7b291..c4d4fb8bc0d 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/hidemethod/HideMethodRefactoring.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/hidemethod/HideMethodRefactoring.java @@ -46,7 +46,7 @@ import org.eclipse.cdt.core.model.ICProject; import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPVisitor; import org.eclipse.cdt.internal.core.pdom.dom.PDOMName; -import org.eclipse.cdt.internal.ui.refactoring.AddDeclarationNodeToClassChange; +import org.eclipse.cdt.internal.ui.refactoring.ClassMemberInserter; import org.eclipse.cdt.internal.ui.refactoring.CRefactoring; import org.eclipse.cdt.internal.ui.refactoring.CRefactoringDescription; import org.eclipse.cdt.internal.ui.refactoring.ModificationCollector; @@ -277,7 +277,7 @@ public class HideMethodRefactoring extends CRefactoring { TextEditGroup editGroup = new TextEditGroup(Messages.HideMethodRefactoring_FILE_CHANGE_TEXT+ methodToHide.getRawSignature()); ICPPASTCompositeTypeSpecifier classDefinition = (ICPPASTCompositeTypeSpecifier) methodToHideDecl.getParent(); - AddDeclarationNodeToClassChange.createChange(classDefinition, VisibilityEnum.v_private, methodToHideDecl, false, collector); + ClassMemberInserter.createChange(classDefinition, VisibilityEnum.v_private, methodToHideDecl, false, collector); rewriter.remove(methodToHideDecl, editGroup); } finally {