From: Raphael Date: Tue, 14 Jul 2009 12:08:02 +0000 (-0400) Subject: ADT XML extract string ID: generate Context.getResources() calls X-Git-Tag: android-x86-2.2~610^2~114 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=40a14ef09e82fdaa0aff9d110c5ab7ff42cb731e;p=android-x86%2Fsdk.git ADT XML extract string ID: generate Context.getResources() calls depending on the Java context of the replacement. In the input page also display the string ID values. --- diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/extractstring/ExtractStringInputPage.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/extractstring/ExtractStringInputPage.java index 286f252ff..a3fdb7dc2 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/extractstring/ExtractStringInputPage.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/extractstring/ExtractStringInputPage.java @@ -44,7 +44,7 @@ import org.eclipse.swt.widgets.Label; import org.eclipse.swt.widgets.Text; import java.util.HashMap; -import java.util.Set; +import java.util.Map; import java.util.TreeSet; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -323,9 +323,12 @@ class ExtractStringInputPage extends UserInputWizardPage implements IWizardPage ref.setTargetFile(resFile); sLastResFilePath.put(mProject.getFullPath().toPortableString(), resFile); - if (mXmlHelper.isResIdDuplicate(mProject, resFile, text)) { - String msg = String.format("There's already a string item called '%1$s' in %2$s.", - text, resFile); + String idValue = mXmlHelper.valueOfStringId(mProject, resFile, text); + if (idValue != null) { + String msg = String.format("%1$s already contains a string ID '%2$s' with value '%3$s'.", + resFile, + text, + idValue); if (ref.getMode() == ExtractStringRefactoring.Mode.SELECT_NEW_ID) { setErrorMessage(msg); success = false; @@ -353,14 +356,14 @@ class ExtractStringInputPage extends UserInputWizardPage implements IWizardPage private void updateStringValueCombo() { String resFile = mResFileCombo.getText(); - Set ids = mXmlHelper.getResIdsForFile(mProject, resFile); + Map ids = mXmlHelper.getResIdsForFile(mProject, resFile); // get the current text from the combo, to make sure we don't change it String currText = mStringIdCombo.getText(); // erase the choices and fill with the given ids mStringIdCombo.removeAll(); - mStringIdCombo.setItems(ids.toArray(new String[ids.size()])); + mStringIdCombo.setItems(ids.keySet().toArray(new String[ids.size()])); // set the current text to preserve it in case it changed if (!currText.equals(mStringIdCombo.getText())) { diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/extractstring/ExtractStringRefactoring.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/extractstring/ExtractStringRefactoring.java index 17d3256db..348d0d799 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/extractstring/ExtractStringRefactoring.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/extractstring/ExtractStringRefactoring.java @@ -51,11 +51,22 @@ import org.eclipse.jdt.core.dom.AST; import org.eclipse.jdt.core.dom.ASTNode; import org.eclipse.jdt.core.dom.ASTParser; import org.eclipse.jdt.core.dom.ASTVisitor; +import org.eclipse.jdt.core.dom.ClassInstanceCreation; import org.eclipse.jdt.core.dom.CompilationUnit; +import org.eclipse.jdt.core.dom.IMethodBinding; +import org.eclipse.jdt.core.dom.ITypeBinding; +import org.eclipse.jdt.core.dom.MethodDeclaration; +import org.eclipse.jdt.core.dom.MethodInvocation; import org.eclipse.jdt.core.dom.Name; -import org.eclipse.jdt.core.dom.QualifiedName; import org.eclipse.jdt.core.dom.SimpleName; +import org.eclipse.jdt.core.dom.SimpleType; +import org.eclipse.jdt.core.dom.SingleVariableDeclaration; import org.eclipse.jdt.core.dom.StringLiteral; +import org.eclipse.jdt.core.dom.Type; +import org.eclipse.jdt.core.dom.TypeDeclaration; +import org.eclipse.jdt.core.dom.VariableDeclarationExpression; +import org.eclipse.jdt.core.dom.VariableDeclarationFragment; +import org.eclipse.jdt.core.dom.VariableDeclarationStatement; import org.eclipse.jdt.core.dom.rewrite.ASTRewrite; import org.eclipse.jdt.core.dom.rewrite.ImportRewrite; import org.eclipse.jface.text.ITextSelection; @@ -112,7 +123,7 @@ import java.util.Map; *
  • On success, the wizard is shown, which let the user input the new ID to use. *
  • The wizard sets the user input values into this refactoring instance, e.g. the new string * ID, the XML file to update, etc. The wizard does use the utility method - * {@link XmlStringFileHelper#isResIdDuplicate(IProject, String, String)} to check whether + * {@link XmlStringFileHelper#valueOfStringId(IProject, String, String)} to check whether * the new ID is already defined in the target XML file. *
  • Once Preview or Finish is selected in the wizard, the * {@link #checkFinalConditions(IProgressMonitor)} is called to double-check the user input @@ -779,7 +790,7 @@ public class ExtractStringRefactoring extends Refactoring { // Prepare the change for the XML file. - if (!mXmlHelper.isResIdDuplicate(mProject, mTargetXmlFileWsPath, mXmlStringId)) { + if (mXmlHelper.valueOfStringId(mProject, mTargetXmlFileWsPath, mXmlStringId) != null) { // We actually change it only if the ID doesn't exist yet Change change = createXmlChange((IFile) targetXml, mXmlStringId, mXmlStringValue, status, SubMonitor.convert(monitor, 1)); @@ -1381,20 +1392,280 @@ public class ExtractStringRefactoring extends Refactoring { mXmlId = xmlId; } + @SuppressWarnings("unchecked") @Override public boolean visit(StringLiteral node) { if (node.getLiteralValue().equals(mOldString)) { - Name qualifierName = mAst.newName(mRQualifier + ".string"); //$NON-NLS-1$ + // We want to analyze the calling context to understand whether we can + // just replace the string literal by the named int constant (R.id.foo) + // or if we should generate a Context.getResources().getString() call. + boolean useGetResource = false; + useGetResource = examineVariableDeclaration(node) || + examineMethodInvocation(node); + + Name qualifierName = mAst.newName(mRQualifier + ".string"); //$NON-NLS-1$ SimpleName idName = mAst.newSimpleName(mXmlId); - QualifiedName newNode = mAst.newQualifiedName(qualifierName, idName); + ASTNode newNode = mAst.newQualifiedName(qualifierName, idName); + String title = "Replace string by ID"; + + if (useGetResource) { + + MethodInvocation mi1 = mAst.newMethodInvocation(); + mi1.setName(mAst.newSimpleName("getResources")); //$NON-NLS-1$ + + String context = methodHasContextArgument(node); + if (context == null && !isClassDerivedFromContext(node)) { + // if we don't have a class that derives from Context and + // we don't have a Context method argument, then make it + // write Context.getResources(), which is technically invalid + // but makes it a good clue on how to fix it. + context = "Context"; //$NON-NLS-1$ + } + if (context != null) { + mi1.setExpression(mAst.newSimpleName(context)); + } - TextEditGroup editGroup = new TextEditGroup("Replace string by ID"); + MethodInvocation mi2 = mAst.newMethodInvocation(); + mi2.setName(mAst.newSimpleName("getString")); //$NON-NLS-1$ + mi2.setExpression(mi1); + mi2.arguments().add(newNode); + + newNode = mi2; + title = "Replace string by getResource().getString(ID)"; + } + + TextEditGroup editGroup = new TextEditGroup(title); mEditGroups.add(editGroup); mRewriter.replace(node, newNode, editGroup); } return super.visit(node); } + + /** + * Examines if the StringLiteral is part of of an assignment to a string, + * e.g. String foo = id. + * + * The parent fragment is of syntax "var = expr" or "var[] = expr". + * We want the type of the variable, which is either held by a + * VariableDeclarationStatement ("type [fragment]") or by a + * VariableDeclarationExpression. In either case, the type can be an array + * but for us all that matters is to know whether the type is an int or + * a string. + */ + private boolean examineVariableDeclaration(StringLiteral node) { + VariableDeclarationFragment fragment = findParentClass(node, + VariableDeclarationFragment.class); + + if (fragment != null) { + ASTNode parent = fragment.getParent(); + + Type type = null; + if (parent instanceof VariableDeclarationStatement) { + type = ((VariableDeclarationStatement) parent).getType(); + } else if (parent instanceof VariableDeclarationExpression) { + type = ((VariableDeclarationExpression) parent).getType(); + } + + if (type instanceof SimpleType) { + return isStringType(type.resolveBinding()); + } + } + + return false; + } + + /** + * If the expression is part of a method invocation (aka a function call) or a + * class instance creation (aka a "new SomeClass" constructor call), we try to + * find the type of the argument being used. If it is a String (most likely), we + * want to return true (to generate a getResources() call). However if there might + * be a similar method that takes an int, in which case we don't want to do that. + * + * This covers the case of Activity.setTitle(int resId) vs setTitle(String str). + */ + @SuppressWarnings("unchecked") + private boolean examineMethodInvocation(StringLiteral node) { + + ASTNode parent = null; + List arguments = null; + IMethodBinding methodBinding = null; + + MethodInvocation invoke = findParentClass(node, MethodInvocation.class); + if (invoke != null) { + parent = invoke; + arguments = invoke.arguments(); + methodBinding = invoke.resolveMethodBinding(); + } else { + ClassInstanceCreation newclass = findParentClass(node, ClassInstanceCreation.class); + if (newclass != null) { + parent = newclass; + arguments = newclass.arguments(); + methodBinding = newclass.resolveConstructorBinding(); + } + } + + if (parent != null && arguments != null && methodBinding != null) { + // We want to know which argument this is. + // Walk up the hierarchy again to find the immediate child of the parent, + // which should turn out to be one of the invocation arguments. + ASTNode child = null; + for (ASTNode n = node; n != parent; ) { + ASTNode p = n.getParent(); + if (p == parent) { + child = n; + break; + } + n = p; + } + if (child == null) { + // This can't happen: a parent of 'node' must be the child of 'parent'. + return false; + } + + // Find the index + int index = 0; + for (Object arg : arguments) { + if (arg == child) { + break; + } + index++; + } + + if (index == arguments.size()) { + // This can't happen: one of the arguments of 'invoke' must be 'child'. + return false; + } + + // Eventually we want to determine if the parameter is a string type, + // in which case a Context.getResources() call must be generated. + boolean useStringType = false; + + // Find the type of that argument + ITypeBinding[] types = methodBinding.getParameterTypes(); + if (index < types.length) { + ITypeBinding type = types[index]; + useStringType = isStringType(type); + } + + // Now that we know that this method takes a String parameter, can we find + // a variant that would accept an int for the same parameter position? + if (useStringType) { + String name = methodBinding.getName(); + ITypeBinding clazz = methodBinding.getDeclaringClass(); + nextMethod: for (IMethodBinding mb2 : clazz.getDeclaredMethods()) { + if (methodBinding == mb2 || !mb2.getName().equals(name)) { + continue; + } + // We found a method with the same name. We want the same parameters + // except that the one at 'index' must be an int type. + ITypeBinding[] types2 = mb2.getParameterTypes(); + int len2 = types2.length; + if (types.length == len2) { + for (int i = 0; i < len2; i++) { + if (i == index) { + ITypeBinding type2 = types2[i]; + if (!("int".equals(type2.getQualifiedName()))) { //$NON-NLS-1$ + // The argument at 'index' is not an int. + continue nextMethod; + } + } else if (!types[i].equals(types2[i])) { + // One of the other arguments do not match our original method + continue nextMethod; + } + } + // If we got here, we found a perfect match: a method with the same + // arguments except the one at 'index' is an int. In this case we + // don't need to convert our R.id into a string. + useStringType = false; + break; + } + } + } + + return useStringType; + } + return false; + } + + /** + * Examines if the StringLiteral is part of a method declaration (a.k.a. a function + * definition) which takes a Context argument. + * If such, it returns the name of the variable. + * Otherwise it returns null. + */ + private String methodHasContextArgument(StringLiteral node) { + MethodDeclaration decl = findParentClass(node, MethodDeclaration.class); + if (decl != null) { + for (Object obj : decl.parameters()) { + if (obj instanceof SingleVariableDeclaration) { + SingleVariableDeclaration var = (SingleVariableDeclaration) obj; + if (isAndroidContext(var.getType())) { + return var.getName().getIdentifier(); + } + } + } + } + return null; + } + + /** + * Returns true if this type binding represents a String or CharSequence type. + */ + private boolean isStringType(ITypeBinding type) { + if (type == null) { + return false; + } + return + "java.lang.String".equals(type.getQualifiedName()) || //$NON-NLS-1$ + "java.lang.CharSequence".equals(type.getQualifiedName()); //$NON-NLS-1$ + } + + /** + * Walks up the node hierarchy and returns the first ASTNode of the requested class. + * Only look at parents. + * + * Implementation note: this is a generic method so that it returns the node already + * casted to the requested type. + */ + @SuppressWarnings("unchecked") + private T findParentClass(ASTNode node, Class clazz) { + for (node = node.getParent(); node != null; node = node.getParent()) { + if (node.getClass().equals(clazz)) { + return (T) node; + } + } + return null; + } + + /** + * Walks up the node hierarchy to find the class (aka type) where this statement + * is used and returns true if this class derives from android.content.Context. + */ + private boolean isClassDerivedFromContext(StringLiteral node) { + TypeDeclaration parent = findParentClass(node, TypeDeclaration.class); + if (parent != null) { + return isAndroidContext(parent.getSuperclassType()); + } + return false; + } + + /** + * Returns true if the given type is or derives from android.content.Context. + */ + private boolean isAndroidContext(Type type) { + if (type != null) { + ITypeBinding binding = type.resolveBinding(); + if (binding != null && binding.isClass()) { + for(; binding != null; binding = binding.getSuperclass()) { + if (binding.getQualifiedName().equals("android.content.Context")) { //$NON-NLS-1$ + return true; + } + } + } + } + return false; + } } /** diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/extractstring/XmlStringFileHelper.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/extractstring/XmlStringFileHelper.java index 6a1beebeb..2dadf27f7 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/extractstring/XmlStringFileHelper.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/refactorings/extractstring/XmlStringFileHelper.java @@ -22,12 +22,14 @@ import org.eclipse.core.resources.IFile; import org.eclipse.core.resources.IProject; import org.eclipse.core.resources.IResource; import org.eclipse.core.runtime.CoreException; +import org.w3c.dom.NamedNodeMap; +import org.w3c.dom.Node; import org.w3c.dom.NodeList; import org.xml.sax.InputSource; import java.util.HashMap; -import java.util.Set; -import java.util.TreeSet; +import java.util.Map; +import java.util.TreeMap; import javax.xml.xpath.XPath; import javax.xml.xpath.XPathConstants; @@ -39,8 +41,12 @@ import javax.xml.xpath.XPathExpressionException; class XmlStringFileHelper { /** A temporary cache of R.string IDs defined by a given xml file. The key is the - * project path of the file, the data is a set of known string Ids for that file. */ - private HashMap> mResIdCache = new HashMap>(); + * project path of the file, the data is a set of known string Ids for that file. + * + * Map type: map [String filename] => map [String id => String value]. + */ + private HashMap> mResIdCache = + new HashMap>(); /** An instance of XPath, created lazily on demand. */ private XPath mXPath; @@ -48,18 +54,18 @@ class XmlStringFileHelper { } /** - * Utility method used by the wizard to check whether the given string ID is already - * defined in the XML file which path is given. + * Utility method used by the wizard to retrieve the actual value definition of a given + * string ID. * * @param project The project contain the XML file. * @param xmlFileWsPath The project path of the XML file, e.g. "/res/values/strings.xml". * The given file may or may not exist. * @param stringId The string ID to find. - * @return True if such a string ID is already defined. + * @return The value string if the ID is defined, null otherwise. */ - public boolean isResIdDuplicate(IProject project, String xmlFileWsPath, String stringId) { - Set cache = getResIdsForFile(project, xmlFileWsPath); - return cache.contains(stringId); + public String valueOfStringId(IProject project, String xmlFileWsPath, String stringId) { + Map cache = getResIdsForFile(project, xmlFileWsPath); + return cache.get(stringId); } /** @@ -70,10 +76,10 @@ class XmlStringFileHelper { * @param project The project contain the XML file. * @param xmlFileWsPath The project path of the XML file, e.g. "/res/values/strings.xml". * The given file may or may not exist. - * @return The set of string IDs defined in the given file. Cached. Never null. + * @return The map of string IDs => values defined in the given file. Cached. Never null. */ - public Set getResIdsForFile(IProject project, String xmlFileWsPath) { - Set cache = mResIdCache.get(xmlFileWsPath); + public Map getResIdsForFile(IProject project, String xmlFileWsPath) { + Map cache = mResIdCache.get(xmlFileWsPath); if (cache == null) { cache = internalGetResIdsForFile(project, xmlFileWsPath); mResIdCache.put(xmlFileWsPath, cache); @@ -85,11 +91,11 @@ class XmlStringFileHelper { * Extract all the defined string IDs from a given file using XPath. * @param project The project contain the XML file. * @param xmlFileWsPath The project path of the file to parse. It may not exist. - * @return The set of all string IDs defined in the file. The returned set is always non - * null. It is empty if the file does not exist. + * @return The map of all string IDs => values defined in the file. + * The returned set is always non null. It is empty if the file does not exist. */ - private Set internalGetResIdsForFile(IProject project, String xmlFileWsPath) { - TreeSet ids = new TreeSet(); + private Map internalGetResIdsForFile(IProject project, String xmlFileWsPath) { + TreeMap ids = new TreeMap(); if (mXPath == null) { mXPath = AndroidXPathFactory.newXPath(); @@ -108,20 +114,26 @@ class XmlStringFileHelper { // something // - String xpathExpr = "/resources/string/@name"; //$NON-NLS-1$ + String xpathExpr = "/resources/string"; //$NON-NLS-1$ Object result = mXPath.evaluate(xpathExpr, source, XPathConstants.NODESET); if (result instanceof NodeList) { NodeList list = (NodeList) result; for (int n = list.getLength() - 1; n >= 0; n--) { - String id = list.item(n).getNodeValue(); - ids.add(id); + Node strNode = list.item(n); + NamedNodeMap attrs = strNode.getAttributes(); + Node nameAttr = attrs.getNamedItem("name"); //$NON-NLS-1$ + if (nameAttr != null) { + String id = nameAttr.getNodeValue(); + String text = strNode.getTextContent(); + ids.put(id, text); + } } } } catch (CoreException e1) { // IFile.getContents failed. Ignore. - } catch (XPathExpressionException e) { + } catch (XPathExpressionException e2) { // mXPath.evaluate failed. Ignore. } }