From 3a4684e8f2afa86722ce4020ef529ed664f0b22d Mon Sep 17 00:00:00 2001 From: Dmitry Cheryasov Date: Tue, 28 Apr 2009 04:56:06 +0400 Subject: [PATCH] Makes 'as' clauses in autoimport avoid unwanted name redefinitions. Removes '__init__.py' from autoimport candidates. No tests for these yet due to stub index not being available at test time; to be added later. Fixed stupid error in previous autoimport tests that gave false positives. --- python/src/com/jetbrains/python/PyNames.java | 1 + .../PyUnresolvedReferencesInspection.java | 48 +++++++++++-------- .../python/psi/resolve/ResolveImportUtil.java | 6 ++- .../inspections/AddToImportFromList_after.py | 2 +- .../inspections/QualifyByImport_after.py | 3 ++ .../com/jetbrains/python/QuickFixTest.java | 7 +-- 6 files changed, 41 insertions(+), 26 deletions(-) create mode 100644 python/testData/inspections/QualifyByImport_after.py diff --git a/python/src/com/jetbrains/python/PyNames.java b/python/src/com/jetbrains/python/PyNames.java index 5e5df73cd60a..7580b020f29d 100644 --- a/python/src/com/jetbrains/python/PyNames.java +++ b/python/src/com/jetbrains/python/PyNames.java @@ -15,6 +15,7 @@ public class PyNames { @NonNls public static final String INIT = "__init__"; @NonNls public static final String DOT_PY = ".py"; + @NonNls public static final String INIT_DOT_PY = INIT + DOT_PY; @NonNls public static final String OBJECT = "object"; @NonNls public static final String NONE = "None"; diff --git a/python/src/com/jetbrains/python/inspections/PyUnresolvedReferencesInspection.java b/python/src/com/jetbrains/python/inspections/PyUnresolvedReferencesInspection.java index 997847cf5158..9a3b84d51666 100644 --- a/python/src/com/jetbrains/python/inspections/PyUnresolvedReferencesInspection.java +++ b/python/src/com/jetbrains/python/inspections/PyUnresolvedReferencesInspection.java @@ -2,7 +2,6 @@ package com.jetbrains.python.inspections; import com.intellij.codeHighlighting.HighlightDisplayLevel; import com.intellij.codeInspection.*; -import com.intellij.lang.ASTNode; import com.intellij.lang.annotation.HighlightSeverity; import com.intellij.openapi.project.Project; import com.intellij.openapi.vfs.VirtualFile; @@ -11,7 +10,6 @@ import com.intellij.psi.search.GlobalSearchScope; import com.intellij.psi.stubs.StubIndex; import com.intellij.psi.util.PsiTreeUtil; import com.jetbrains.python.PyBundle; -import com.jetbrains.python.PyTokenTypes; import com.jetbrains.python.actions.AddFieldQuickFix; import com.jetbrains.python.actions.AddImportAction; import com.jetbrains.python.actions.AddMethodQuickFix; @@ -78,12 +76,18 @@ public class PyUnresolvedReferencesInspection extends LocalInspectionTool { super(holder); } + static String getFirstQualifier(String name) { + // foo.bar.baz -> foo + int pos = name.indexOf('.'); + if (pos > 0) return name.substring(0, pos); + else return name; + } + @Nullable static HintAction proposeImportFixes(final PyElement node, String ref_text) { boolean worthy_fix = false; ImportFromExistingFix fix = null; Set seen_file_names = new HashSet(); // true import names - Set seen_as_names = new HashSet(); // 'as' parts: we don't want to clas with them either // maybe the name is importable via some exisitng 'import foo' statement, and only needs a qualifier. // walk up collecting all such statements and analyzing CollectProcessor import_prc = new CollectProcessor(PyImportStatement.class); @@ -98,8 +102,8 @@ public class PyUnresolvedReferencesInspection extends LocalInspectionTool { PsiElement dst = src.resolve(); if (dst instanceof PyFile) { PyFile dst_file = (PyFile)dst; - seen_file_names.add(ielt.getImportReference().getReferencedName()); // ref is ok or matching would fail - seen_as_names.add(ielt.getVisibleName()); + String name = ielt.getImportReference().getReferencedName(); // ref is ok or matching would fail + seen_file_names.add(name); PsiElement res = (dst_file).findExportedName(ref_text); if (res != null) { fix.addImport(res, dst_file, ielt); @@ -126,7 +130,8 @@ public class PyUnresolvedReferencesInspection extends LocalInspectionTool { PsiElement dst = src.resolve(); if (dst instanceof PyFile) { PyFile dst_file = (PyFile)dst; - seen_file_names.add(from_stmt.getImportSource().getReferencedName()); // source is ok, else it won't match and we'd not be adding it + String name = from_stmt.getImportSource().getReferencedName(); // source is ok, else it won't match and we'd not be adding it + seen_file_names.add(name); PsiElement res = (dst_file).findExportedName(ref_text); if (res != null) { fix.addImport(res, dst_file, ielts[ielts.length-1]); // last element; action expects to add to tail @@ -155,14 +160,9 @@ public class PyUnresolvedReferencesInspection extends LocalInspectionTool { String import_path = ResolveImportUtil.findShortestImportableName(node, vfile); if (import_path != null && !seen_file_names.contains(import_path)) { // a new, valid hit - String as_name = null; - if (seen_as_names.contains(import_path)) { - // an 'as' name somewhere above eclipses the true name. get us a unique 'as' name. - as_name = propseAsName(node.getContainingFile(), import_path); - seen_as_names.add(as_name); // just in case - } - fix.addImport(symbol, srcfile, null, import_path, as_name); + fix.addImport(symbol, srcfile, null, import_path, propseAsName(node.getContainingFile(), ref_text, import_path)); seen_file_names.add(import_path); // just in case, again + worthy_fix = true; } } } @@ -177,20 +177,26 @@ public class PyUnresolvedReferencesInspection extends LocalInspectionTool { private final static String[] AS_PREFIXES = {"other_", "one_more_", "different_", "pseudo_", "true_"}; // a no-frills recursive accumulating scan - private static void collectIdentifiers(ASTNode node, Collection dst) { - ASTNode seeker = node.getFirstChildNode(); + private static void collectIdentifiers(PsiElement node, Collection dst) { + PsiElement seeker = node.getFirstChild(); while (seeker != null) { - if (seeker.getElementType() == PyTokenTypes.IDENTIFIER) dst.add(seeker.getText()); + if (seeker instanceof NameDefiner) { + for (PyElement named : ((NameDefiner)seeker).iterateNames()) { + if (named != null) dst.add(named.getName()); + } + } else collectIdentifiers(seeker, dst); - seeker = seeker.getTreeNext(); + seeker = seeker.getNextSibling(); } } - // find an unique name that does not clash with anything in the file, using ref_test and import_path as hints - private static String propseAsName(PsiFile file, String import_path) { + // find an unique name that does not clash with anything in the file, using ref_name and import_path as hints + private static String propseAsName(PsiFile file, String ref_name, String import_path) { // a somehow brute-force approach: collect all identifiers wholesale and avoid clashes with any of them Set ident_set = new HashSet(); - collectIdentifiers(file.getNode(), ident_set); + collectIdentifiers(file, ident_set); + // try the default, 'normal' name first; if it does not clash, propose no sustitute! + if (! ident_set.contains(ref_name)) return null; // try flattened import path String path_name = import_path.replace('.', '_'); if (! ident_set.contains(path_name)) return path_name; @@ -199,7 +205,7 @@ public class PyUnresolvedReferencesInspection extends LocalInspectionTool { String variant = prefix + path_name; if (! ident_set.contains(variant)) return variant; } - // if nothing helped, just bluntly add a number to the end. guaranteed to finish in ident_set.size() iterations. + // if nothing helped, just bluntly add a number to the end. guaranteed to finish in ident_set.size()+1 iterations. int cnt = 1; while (cnt < Integer.MAX_VALUE) { String variant = path_name + Integer.toString(cnt); diff --git a/python/src/com/jetbrains/python/psi/resolve/ResolveImportUtil.java b/python/src/com/jetbrains/python/psi/resolve/ResolveImportUtil.java index 2fcae44617e8..6bac3e40d66b 100644 --- a/python/src/com/jetbrains/python/psi/resolve/ResolveImportUtil.java +++ b/python/src/com/jetbrains/python/psi/resolve/ResolveImportUtil.java @@ -12,6 +12,7 @@ import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.*; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.util.containers.HashSet; +import com.jetbrains.python.PyNames; import com.jetbrains.python.psi.*; import com.jetbrains.python.psi.impl.PyImportResolver; import org.jetbrains.annotations.NonNls; @@ -560,10 +561,13 @@ public class ResolveImportUtil { private PathChoosingVisitor(VirtualFile file) { myFile = file; - // cut off the ext myFname = file.getPath(); + // cut off the ext int pos = myFname.lastIndexOf('.'); if (pos > 0) myFname = myFname.substring(0, pos); + // cut off the final __init__ if it's there; we want imports directly from a module + pos = myFname.lastIndexOf(PyNames.INIT); + if (pos > 0) myFname = myFname.substring(0, pos-1); // pos-1 also cuts the '/' that came before "__init__" } public boolean visitRoot(VirtualFile root) { diff --git a/python/testData/inspections/AddToImportFromList_after.py b/python/testData/inspections/AddToImportFromList_after.py index ad6b4b707e87..68a2fc4de61f 100644 --- a/python/testData/inspections/AddToImportFromList_after.py +++ b/python/testData/inspections/AddToImportFromList_after.py @@ -1,3 +1,3 @@ -from AddToImportFromFoo import bar, foo +from AddToImportFromFoo import bar,foo foo # must get imported diff --git a/python/testData/inspections/QualifyByImport_after.py b/python/testData/inspections/QualifyByImport_after.py new file mode 100644 index 000000000000..7718df735eeb --- /dev/null +++ b/python/testData/inspections/QualifyByImport_after.py @@ -0,0 +1,3 @@ +import QualifyByImportFoo + +QualifyByImportFoo.foo # must be qualified diff --git a/python/testSrc/com/jetbrains/python/QuickFixTest.java b/python/testSrc/com/jetbrains/python/QuickFixTest.java index b6949259763d..05dd423d93f9 100644 --- a/python/testSrc/com/jetbrains/python/QuickFixTest.java +++ b/python/testSrc/com/jetbrains/python/QuickFixTest.java @@ -32,19 +32,20 @@ public class QuickFixTest extends DaemonAnalyzerTestCase { public void testQualifyByImport() throws Exception { doInspectionTest( new String[]{"QualifyByImport.py", "QualifyByImportFoo.py"}, - PyUnresolvedReferencesInspection.class, "QualifyByImportFoo.foo?", true, false + PyUnresolvedReferencesInspection.class, PyBundle.message("ACT.NAME.use.import"), true, true ); } public void testAddToImportFromList() throws Exception { doInspectionTest( new String[]{"AddToImportFromList.py", "AddToImportFromFoo.py"}, - PyUnresolvedReferencesInspection.class, "foo(a) from AddToImportFromFoo?", true, false + PyUnresolvedReferencesInspection.class, PyBundle.message("ACT.NAME.use.import"), true, true ); } - // TODO: add a test for multiple variants of above + // TODO: add tests for stub indexes-based autoimport of unimported somehow. + public void testAddSelf() throws Exception { doInspectionTest("AddSelf.py", PyMethodParametersInspection.class, PyBundle.message("QFIX.add.parameter.self"), true, true); }