mirror of
https://github.com/eclipse-cdt/cdt
synced 2025-04-29 19:45:01 +02:00
Bug 312736: extract variable refactoring does not infer type or replace expression correctly
https://bugs.eclipse.org/bugs/show_bug.cgi?id=312736
This commit is contained in:
Emanuel Graf
parent
2d7b307308
commit
eea33f080c
4 changed files with 386 additions and 9 deletions
|
|
@ -0,0 +1,49 @@
|
|||
/*******************************************************************************
|
||||
* Copyright (c) 2010 Tomasz Wesolowski
|
||||
* 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:
|
||||
* Tomasz Wesolowski - initial API and implementation
|
||||
*******************************************************************************/
|
||||
|
||||
package org.eclipse.cdt.core.dom.rewrite;
|
||||
|
||||
import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTDeclarator;
|
||||
import org.eclipse.cdt.core.dom.ast.INodeFactory;
|
||||
import org.eclipse.cdt.core.dom.ast.IType;
|
||||
import org.eclipse.cdt.internal.core.dom.rewrite.DeclarationGeneratorImpl;
|
||||
|
||||
/**
|
||||
*
|
||||
* This class handles the creation of {@link IASTDeclarator}s and {@link IASTDeclSpecifier}s basing on given type.
|
||||
*
|
||||
* @author Tomasz Wesolowski
|
||||
* @noextend This interface is not intended to be extended by clients.
|
||||
* @since 5.3
|
||||
*/
|
||||
public abstract class DeclarationGenerator {
|
||||
|
||||
public static DeclarationGenerator create(INodeFactory factory) {
|
||||
return new DeclarationGeneratorImpl(factory);
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a new {@link IASTDeclSpecifier}DeclSpecifier for a given {@link IType}.
|
||||
* @param type the type to describe
|
||||
* @return the generated declaration specifier
|
||||
*/
|
||||
public abstract IASTDeclSpecifier createDeclSpecFromType(IType type);
|
||||
|
||||
/**
|
||||
* Creates a new {@link IASTDeclarator}Declarator for a given {@link IType}.
|
||||
* @param type the type to describe
|
||||
* @param name the name for the declarator
|
||||
* @return the generated declarator
|
||||
*/
|
||||
public abstract IASTDeclarator createDeclaratorFromType(IType type, char[] name);
|
||||
|
||||
}
|
||||
|
|
@ -0,0 +1,255 @@
|
|||
/*******************************************************************************
|
||||
* Copyright (c) 2010 Tomasz Wesolowski
|
||||
* 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:
|
||||
* Tomasz Wesolowski - initial API and implementation
|
||||
*******************************************************************************/
|
||||
|
||||
package org.eclipse.cdt.internal.core.dom.rewrite;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
|
||||
import org.eclipse.cdt.core.dom.ast.DOMException;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTArrayDeclarator;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTArrayModifier;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTDeclarator;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTExpression;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTName;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTNamedTypeSpecifier;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTPointer;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTPointerOperator;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclSpecifier;
|
||||
import org.eclipse.cdt.core.dom.ast.IArrayType;
|
||||
import org.eclipse.cdt.core.dom.ast.IBasicType;
|
||||
import org.eclipse.cdt.core.dom.ast.IBasicType.Kind;
|
||||
import org.eclipse.cdt.core.dom.ast.IBinding;
|
||||
import org.eclipse.cdt.core.dom.ast.ICompositeType;
|
||||
import org.eclipse.cdt.core.dom.ast.IFunctionType;
|
||||
import org.eclipse.cdt.core.dom.ast.INodeFactory;
|
||||
import org.eclipse.cdt.core.dom.ast.IPointerType;
|
||||
import org.eclipse.cdt.core.dom.ast.IQualifierType;
|
||||
import org.eclipse.cdt.core.dom.ast.IType;
|
||||
import org.eclipse.cdt.core.dom.ast.ITypedef;
|
||||
import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTQualifiedName;
|
||||
import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTReferenceOperator;
|
||||
import org.eclipse.cdt.core.dom.ast.cpp.ICPPNodeFactory;
|
||||
import org.eclipse.cdt.core.dom.ast.cpp.ICPPReferenceType;
|
||||
import org.eclipse.cdt.core.dom.rewrite.DeclarationGenerator;
|
||||
import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTArrayDeclarator;
|
||||
import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTDeclarator;
|
||||
import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPVisitor;
|
||||
|
||||
/**
|
||||
*
|
||||
* @author Tomasz Wesolowski
|
||||
*
|
||||
*/
|
||||
public class DeclarationGeneratorImpl extends DeclarationGenerator {
|
||||
|
||||
/* (non-Javadoc)
|
||||
* @see org.eclipse.cdt.core.dom.rewrite.IDeclarationGenerator#createDeclSpecFromType(org.eclipse.cdt.core.dom.ast.IType)
|
||||
*/
|
||||
|
||||
private INodeFactory factory;
|
||||
|
||||
/**
|
||||
* Creates a new generator using the given factory.
|
||||
* @param factory a factory to use. If a C++ type is requested, it has to be an instance of {@link ICPPNodeFactory}.
|
||||
*/
|
||||
public DeclarationGeneratorImpl(INodeFactory factory) {
|
||||
this.factory = factory;
|
||||
}
|
||||
|
||||
@Override
|
||||
public IASTDeclSpecifier createDeclSpecFromType(IType type) {
|
||||
|
||||
IASTDeclSpecifier returnedDeclSpec = null;
|
||||
|
||||
if (type instanceof IPointerType) {
|
||||
IType inner = ((IPointerType)type).getType();
|
||||
returnedDeclSpec = createDeclSpecFromType(inner);
|
||||
} else if (type instanceof ICPPReferenceType) {
|
||||
IType inner = ((ICPPReferenceType)type).getType();
|
||||
returnedDeclSpec = createDeclSpecFromType(inner);
|
||||
} else if (type instanceof IArrayType) {
|
||||
IType inner = ((IArrayType)type).getType();
|
||||
returnedDeclSpec = createDeclSpecFromType(inner);
|
||||
} else if (type instanceof IFunctionType) {
|
||||
IType inner = ((IFunctionType)type).getReturnType();
|
||||
returnedDeclSpec = createDeclSpecFromType(inner);
|
||||
} else if (type instanceof IQualifierType) {
|
||||
IType inner = ((IQualifierType)type).getType();
|
||||
returnedDeclSpec = createDeclSpecFromType(inner);
|
||||
if (((IQualifierType) type).isConst()) {
|
||||
returnedDeclSpec.setConst(true);
|
||||
}
|
||||
if (((IQualifierType) type).isVolatile()) {
|
||||
returnedDeclSpec.setVolatile(true);
|
||||
}
|
||||
} else if (type instanceof IBasicType) {
|
||||
Kind kind = ((IBasicType)type).getKind();
|
||||
IASTSimpleDeclSpecifier declSpec = factory.newSimpleDeclSpecifier();
|
||||
declSpec.setType(kind);
|
||||
returnedDeclSpec = declSpec;
|
||||
} else if (type instanceof IBinding) { /* ITypedef, ICompositeType... */
|
||||
// BTW - we need to distinguish (and fail explicitly) on literal composites like:
|
||||
// struct { } aSingleInstance;
|
||||
returnedDeclSpec = getDeclSpecForBinding((IBinding) type);
|
||||
}
|
||||
|
||||
// Fallback...
|
||||
if (returnedDeclSpec == null) {
|
||||
IASTSimpleDeclSpecifier specifier = factory.newSimpleDeclSpecifier();
|
||||
specifier.setType(Kind.eVoid);
|
||||
returnedDeclSpec = specifier;
|
||||
}
|
||||
return returnedDeclSpec;
|
||||
}
|
||||
|
||||
/* (non-Javadoc)
|
||||
* @see org.eclipse.cdt.core.dom.rewrite.IDeclarationGenerator#createDeclaratorFromType(org.eclipse.cdt.core.dom.ast.IType, char[])
|
||||
*/
|
||||
@Override
|
||||
public IASTDeclarator createDeclaratorFromType(IType type, char[] name) {
|
||||
|
||||
IASTDeclarator returnedDeclarator = null;
|
||||
|
||||
try {
|
||||
|
||||
if (type instanceof IPointerType || type instanceof IArrayType || type instanceof ICPPReferenceType) {
|
||||
|
||||
IASTDeclarator declarator = createPointerOrArrayDeclarator(type, name);
|
||||
returnedDeclarator = declarator;
|
||||
|
||||
} else if (typeIrrelevantForDeclarator(type)) {
|
||||
IASTDeclarator declarator = factory.newDeclarator(null);
|
||||
IASTName astName = factory.newName(name);
|
||||
declarator.setName(astName);
|
||||
|
||||
returnedDeclarator = declarator;
|
||||
|
||||
} else if (type instanceof IFunctionType) {
|
||||
// TODO function pointers
|
||||
|
||||
|
||||
}
|
||||
} catch (DOMException e) {
|
||||
e.printStackTrace();
|
||||
}
|
||||
|
||||
// Fallback
|
||||
if (returnedDeclarator == null) {
|
||||
returnedDeclarator = factory.newDeclarator(factory.newName(name));
|
||||
}
|
||||
|
||||
return returnedDeclarator;
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if a given type isn't described by the {@link IASTDeclarator} (only by the {@link IASTDeclSpecifier}).
|
||||
* @param type the type to check
|
||||
* @return true if irrelevant
|
||||
*/
|
||||
private boolean typeIrrelevantForDeclarator(IType type) {
|
||||
return type instanceof IBasicType || type instanceof ICompositeType || type instanceof IQualifierType || type instanceof ITypedef;
|
||||
}
|
||||
|
||||
/**
|
||||
* Handles the creation of declarators of pointers, references, arrays and arrays of pointers/references.
|
||||
* @param type the topmost type, expected {@link IPointerType}, {@link ICPPReferenceType} or {@link IArrayType}
|
||||
* @param name the declarator name
|
||||
* @return Either a {@link CPPASTDeclarator} or a {@link CPPASTArrayDeclarator}, depending on what's needed
|
||||
* @throws DOMException
|
||||
*/
|
||||
private IASTDeclarator createPointerOrArrayDeclarator(IType type, char[] name) throws DOMException {
|
||||
List<IASTPointerOperator> ptrs = new ArrayList<IASTPointerOperator>();
|
||||
List<IASTExpression> arrs = new ArrayList<IASTExpression>();
|
||||
while (type instanceof IPointerType || type instanceof ICPPReferenceType) {
|
||||
IASTPointerOperator astPtr;
|
||||
if (type instanceof IPointerType) {
|
||||
IASTPointer cppastPointer = factory.newPointer();
|
||||
IPointerType ptrType = (IPointerType) type;
|
||||
cppastPointer.setConst(ptrType.isConst());
|
||||
cppastPointer.setVolatile(ptrType.isVolatile());
|
||||
astPtr = cppastPointer;
|
||||
type = ptrType.getType();
|
||||
} else {
|
||||
ICPPASTReferenceOperator cppastReferenceOperator = ((ICPPNodeFactory)factory).newReferenceOperator(false);
|
||||
ICPPReferenceType refType = (ICPPReferenceType) type;
|
||||
astPtr = cppastReferenceOperator;
|
||||
type = refType.getType();
|
||||
|
||||
}
|
||||
ptrs.add(astPtr);
|
||||
}
|
||||
while (type instanceof IArrayType) {
|
||||
IArrayType arrType = (IArrayType) type;
|
||||
IASTExpression arraySizeExpression = arrType.getArraySizeExpression();
|
||||
arrs.add(arraySizeExpression);
|
||||
type = arrType.getType();
|
||||
}
|
||||
|
||||
IASTDeclarator decl;
|
||||
if (!arrs.isEmpty()) {
|
||||
IASTArrayDeclarator arrayDeclarator = factory.newArrayDeclarator(null);
|
||||
for (IASTExpression exp : arrs) {
|
||||
IASTArrayModifier arrayModifier = factory.newArrayModifier((exp == null) ? exp : exp.copy());
|
||||
arrayDeclarator.addArrayModifier(arrayModifier);
|
||||
}
|
||||
decl = arrayDeclarator;
|
||||
} else {
|
||||
decl = factory.newDeclarator(null);
|
||||
}
|
||||
for (IASTPointerOperator ptr : ptrs) {
|
||||
decl.addPointerOperator(ptr);
|
||||
}
|
||||
|
||||
createDeclaratorContent(decl, type, name);
|
||||
|
||||
return decl;
|
||||
}
|
||||
|
||||
/**
|
||||
* Fills a given declarator with either an ASTName or a proper nested declarator
|
||||
* @param decl The declarator to fill
|
||||
* @param type the content type of declarator
|
||||
* @param name the desired name of declarator tree
|
||||
* @throws DOMException
|
||||
*/
|
||||
private void createDeclaratorContent(IASTDeclarator decl, IType type, char[] name) throws DOMException {
|
||||
if (type instanceof IPointerType || type instanceof IArrayType || type instanceof ICPPReferenceType) {
|
||||
IASTDeclarator nested = createPointerOrArrayDeclarator(type, name);
|
||||
decl.setNestedDeclarator(nested);
|
||||
} else if (typeIrrelevantForDeclarator(type)) {
|
||||
IASTName astName = factory.newName(name);
|
||||
decl.setName(astName);
|
||||
}
|
||||
}
|
||||
|
||||
private IASTNamedTypeSpecifier getDeclSpecForBinding(IBinding binding) {
|
||||
|
||||
char[][] qualifiedNameCharArray = CPPVisitor.getQualifiedNameCharArray(binding);
|
||||
if (qualifiedNameCharArray.length > 1) {
|
||||
|
||||
ICPPASTQualifiedName name = ((ICPPNodeFactory)factory).newQualifiedName();
|
||||
for (char[] cs : qualifiedNameCharArray) {
|
||||
name.addName(factory.newName(cs));
|
||||
}
|
||||
return factory.newTypedefNameSpecifier(name);
|
||||
|
||||
} else if (qualifiedNameCharArray.length == 1) {
|
||||
IASTName name = factory.newName(qualifiedNameCharArray[0]);
|
||||
return factory.newTypedefNameSpecifier(name);
|
||||
} else {
|
||||
IASTName name = factory.newName(binding.getName().toCharArray());
|
||||
return factory.newTypedefNameSpecifier(name);
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
|
@ -328,3 +328,75 @@ void foo(){
|
|||
for(int n = i; n < 10; ++n);
|
||||
}
|
||||
|
||||
//!ExtractLocalVariableRefactoringTest expression
|
||||
//#org.eclipse.cdt.ui.tests.refactoring.extractlocalvariable.ExtractLocalVariableRefactoringTest
|
||||
//@A.cpp
|
||||
void foo(){
|
||||
int a = 0;
|
||||
float b = 0.1f;
|
||||
double c = /*$*/(a + b)/*$$*/ * 0.2;
|
||||
}
|
||||
|
||||
//=
|
||||
void foo(){
|
||||
int a = 0;
|
||||
float b = 0.1f;
|
||||
float a0 = a + b;
|
||||
double c = a0 * 0.2;
|
||||
}
|
||||
|
||||
//!ExtractLocalVariableRefactoringTest pointer
|
||||
//#org.eclipse.cdt.ui.tests.refactoring.extractlocalvariable.ExtractLocalVariableRefactoringTest
|
||||
//@A.cpp
|
||||
void foo(){
|
||||
int a[2];
|
||||
int b = */*$*/(a + 1)/*$$*/;
|
||||
}
|
||||
|
||||
//=
|
||||
void foo(){
|
||||
int a[2];
|
||||
int *i = a + 1;
|
||||
int b = *i;
|
||||
}
|
||||
|
||||
//!ExtractLocalVariableRefactoringTest qualifiers
|
||||
//#org.eclipse.cdt.ui.tests.refactoring.extractlocalvariable.ExtractLocalVariableRefactoringTest
|
||||
//@A.cpp
|
||||
const volatile int *k;
|
||||
void foo(){
|
||||
/*$*/k;/*$$*/
|
||||
}
|
||||
|
||||
//=
|
||||
const volatile int *k;
|
||||
void foo(){
|
||||
const volatile int *k0 = k;
|
||||
k0;
|
||||
}
|
||||
|
||||
//!ExtractLocalVariableRefactoringTest overloaded operators
|
||||
//#org.eclipse.cdt.ui.tests.refactoring.extractlocalvariable.ExtractLocalVariableRefactoringTest
|
||||
//@A.cpp
|
||||
class K {
|
||||
public:
|
||||
bool operator+(int b) {return true;}
|
||||
float operator+(unsigned u) {return 1.0f;}
|
||||
};
|
||||
void foo(){
|
||||
K k;
|
||||
/*$*/k+3u/*$$*/;
|
||||
}
|
||||
|
||||
//=
|
||||
class K {
|
||||
public:
|
||||
bool operator+(int b) {return true;}
|
||||
float operator+(unsigned u) {return 1.0f;}
|
||||
};
|
||||
void foo(){
|
||||
K k;
|
||||
float i = k + 3u;
|
||||
i;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -44,19 +44,19 @@ import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration;
|
|||
import org.eclipse.cdt.core.dom.ast.IASTStatement;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTUnaryExpression;
|
||||
import org.eclipse.cdt.core.dom.ast.IBinding;
|
||||
import org.eclipse.cdt.core.dom.ast.INodeFactory;
|
||||
import org.eclipse.cdt.core.dom.ast.IScope;
|
||||
import org.eclipse.cdt.core.dom.ast.cpp.CPPASTVisitor;
|
||||
import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTCompositeTypeSpecifier;
|
||||
import org.eclipse.cdt.core.dom.rewrite.ASTRewrite;
|
||||
import org.eclipse.cdt.core.dom.rewrite.DeclarationGenerator;
|
||||
import org.eclipse.cdt.core.model.ICProject;
|
||||
|
||||
import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTDeclarationStatement;
|
||||
import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTDeclarator;
|
||||
import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTEqualsInitializer;
|
||||
import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTIdExpression;
|
||||
import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTLiteralExpression;
|
||||
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.CPPFunction;
|
||||
|
||||
import org.eclipse.cdt.internal.ui.refactoring.CRefactoring;
|
||||
|
|
@ -64,7 +64,6 @@ import org.eclipse.cdt.internal.ui.refactoring.CRefactoringDescription;
|
|||
import org.eclipse.cdt.internal.ui.refactoring.ModificationCollector;
|
||||
import org.eclipse.cdt.internal.ui.refactoring.NameNVisibilityInformation;
|
||||
import org.eclipse.cdt.internal.ui.refactoring.NodeContainer;
|
||||
import org.eclipse.cdt.internal.ui.refactoring.extractfunction.ExtractExpression;
|
||||
import org.eclipse.cdt.internal.ui.refactoring.utils.NodeHelper;
|
||||
import org.eclipse.cdt.internal.ui.refactoring.utils.SelectionHelper;
|
||||
|
||||
|
|
@ -297,16 +296,18 @@ public class ExtractLocalVariableRefactoring extends CRefactoring {
|
|||
}
|
||||
|
||||
private IASTDeclarationStatement getVariableNodes(String newName) {
|
||||
IASTSimpleDeclaration simple = new CPPASTSimpleDeclaration();
|
||||
|
||||
INodeFactory factory = this.unit.getASTNodeFactory();
|
||||
|
||||
IASTSimpleDeclaration simple = factory.newSimpleDeclaration(null);
|
||||
|
||||
IASTDeclSpecifier declSpec = new ExtractExpression()
|
||||
.determineReturnType(deblock(target), null);
|
||||
DeclarationGenerator generator = DeclarationGenerator.create(factory);
|
||||
|
||||
IASTDeclSpecifier declSpec = generator.createDeclSpecFromType(target.getExpressionType());
|
||||
declSpec.setStorageClass(IASTDeclSpecifier.sc_unspecified);
|
||||
simple.setDeclSpecifier(declSpec);
|
||||
|
||||
IASTDeclarator decl = new CPPASTDeclarator();
|
||||
IASTName name = new CPPASTName(newName.toCharArray());
|
||||
decl.setName(name);
|
||||
IASTDeclarator decl = generator.createDeclaratorFromType(target.getExpressionType(), newName.toCharArray());
|
||||
|
||||
IASTEqualsInitializer init = new CPPASTEqualsInitializer();
|
||||
init.setInitializerClause(deblock(target.copy()));
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue