diff --git a/python/psi-api/src/com/jetbrains/python/PyNames.java b/python/psi-api/src/com/jetbrains/python/PyNames.java index ca7bdadd22ff..a35b3c3f60c5 100644 --- a/python/psi-api/src/com/jetbrains/python/PyNames.java +++ b/python/psi-api/src/com/jetbrains/python/PyNames.java @@ -119,8 +119,8 @@ public class PyNames { public static final String COLLECTIONS = "collections"; public static final String COLLECTIONS_NAMEDTUPLE = COLLECTIONS + "." + NAMEDTUPLE; - public static final String ABSTRACTMETHOD = "abstractmethod"; - public static final String ABSTRACTPROPERTY = "abstractproperty"; + public static final String ABSTRACTMETHOD = "abc.abstractmethod"; + public static final String ABSTRACTPROPERTY = "abc.abstractproperty"; public static final String TUPLE = "tuple"; public static final String SET = "set"; diff --git a/python/src/com/jetbrains/python/inspections/PyAbstractClassInspection.java b/python/src/com/jetbrains/python/inspections/PyAbstractClassInspection.java index 34c09a02b4bd..25bf4a4590b5 100644 --- a/python/src/com/jetbrains/python/inspections/PyAbstractClassInspection.java +++ b/python/src/com/jetbrains/python/inspections/PyAbstractClassInspection.java @@ -6,14 +6,11 @@ import com.intellij.codeInspection.ProblemsHolder; import com.intellij.lang.ASTNode; import com.intellij.psi.PsiElementVisitor; import com.jetbrains.python.PyBundle; -import com.jetbrains.python.PyNames; import com.jetbrains.python.codeInsight.override.PyOverrideImplementUtil; import com.jetbrains.python.inspections.quickfix.PyImplementMethodsQuickFix; import com.jetbrains.python.psi.PyClass; -import com.jetbrains.python.psi.PyDecorator; -import com.jetbrains.python.psi.PyDecoratorList; import com.jetbrains.python.psi.PyFunction; -import com.jetbrains.python.psi.impl.PyQualifiedName; +import com.jetbrains.python.psi.PyUtil; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -51,15 +48,8 @@ public class PyAbstractClassInspection extends PyInspection { Set toBeImplemented = new HashSet(); final Collection functions = PyOverrideImplementUtil.getAllSuperFunctions(node); for (PyFunction method : functions) { - final PyDecoratorList list = method.getDecoratorList(); - if (list != null && node.findMethodByName(method.getName(), false) == null) { - for (PyDecorator decorator : list.getDecorators()) { - final PyQualifiedName qualifiedName = decorator.getQualifiedName(); - if (qualifiedName != null && (PyNames.ABSTRACTMETHOD.equals(qualifiedName.getLastComponent()) || PyNames.ABSTRACTPROPERTY.equals( - qualifiedName.getLastComponent()))) { - toBeImplemented.add(method); - } - } + if (node.findMethodByName(method.getName(), false) == null && PyUtil.isDecoratedAsAbstract(method)) { + toBeImplemented.add(method); } } final ASTNode nameNode = node.getNameNode(); diff --git a/python/src/com/jetbrains/python/inspections/PyMethodMayBeStaticInspection.java b/python/src/com/jetbrains/python/inspections/PyMethodMayBeStaticInspection.java index 706e73cd3935..4e01fa8089a0 100644 --- a/python/src/com/jetbrains/python/inspections/PyMethodMayBeStaticInspection.java +++ b/python/src/com/jetbrains/python/inspections/PyMethodMayBeStaticInspection.java @@ -55,17 +55,9 @@ public class PyMethodMayBeStaticInspection extends PyInspection { if (!supers.isEmpty()) return; final Collection overrides = PyOverridingMethodsSearch.search(node, true).findAll(); if (!overrides.isEmpty()) return; - final PyDecoratorList decoratorList = node.getDecoratorList(); - if (decoratorList != null) { - for (PyDecorator decorator : decoratorList.getDecorators()) { - final String decoratorName = decorator.getName(); - if (hasSpecificDecorator(decoratorName)) { - return; - } - final Property property = containingClass.findPropertyByCallable(node); - if (property != null) return; - } - } + if (PyUtil.isDecoratedAsAbstract(node) || node.getModifier() != null) return; + final Property property = containingClass.findPropertyByCallable(node); + if (property != null) return; final PyStatementList statementList = node.getStatementList(); if (statementList == null) return; @@ -121,10 +113,5 @@ public class PyMethodMayBeStaticInspection extends PyInspection { null, new PyMakeMethodStaticQuickFix(), new PyMakeFunctionFromMethodQuickFix()); } } - - private static boolean hasSpecificDecorator(String decoratorName) { - return PyNames.STATICMETHOD.equals(decoratorName) || PyNames.CLASSMETHOD.equals(decoratorName) || - PyNames.ABSTRACTMETHOD.equals(decoratorName) || PyNames.ABSTRACTPROPERTY.equals(decoratorName); - } } } diff --git a/python/src/com/jetbrains/python/psi/PyUtil.java b/python/src/com/jetbrains/python/psi/PyUtil.java index 13781403a5bc..2f3d9b290363 100644 --- a/python/src/com/jetbrains/python/psi/PyUtil.java +++ b/python/src/com/jetbrains/python/psi/PyUtil.java @@ -678,6 +678,26 @@ public class PyUtil { return false; } + public static boolean isDecoratedAsAbstract(@NotNull final PyDecoratable decoratable) { + final PyDecoratorList decoratorList = decoratable.getDecoratorList(); + if (decoratorList == null) { + return false; + } + for (PyDecorator decorator : decoratorList.getDecorators()) { + final PyExpression callee = decorator.getCallee(); + if (callee instanceof PyReferenceExpression) { + final PsiReference reference = callee.getReference(); + if (reference == null) continue; + final PsiElement resolved = reference.resolve(); + if (resolved instanceof PyQualifiedNameOwner) { + final String name = ((PyQualifiedNameOwner)resolved).getQualifiedName(); + return PyNames.ABSTRACTMETHOD.equals(name) || PyNames.ABSTRACTPROPERTY.equals(name); + } + } + } + return false; + } + public static class KnownDecoratorProviderHolder { public static PyKnownDecoratorProvider[] KNOWN_DECORATOR_PROVIDERS = Extensions.getExtensions(PyKnownDecoratorProvider.EP_NAME); diff --git a/python/testData/inspections/PyMethodMayBeStaticInspection/abc.py b/python/testData/inspections/PyMethodMayBeStaticInspection/abc.py new file mode 100644 index 000000000000..02e48a1bb32f --- /dev/null +++ b/python/testData/inspections/PyMethodMayBeStaticInspection/abc.py @@ -0,0 +1,185 @@ +# Copyright 2007 Google, Inc. All Rights Reserved. +# Licensed to PSF under a Contributor Agreement. + +"""Abstract Base Classes (ABCs) according to PEP 3119.""" + +import types + +from _weakrefset import WeakSet + +# Instance of old-style class +class _C: pass +_InstanceType = type(_C()) + + +def abstractmethod(funcobj): + """A decorator indicating abstract methods. + + Requires that the metaclass is ABCMeta or derived from it. A + class that has a metaclass derived from ABCMeta cannot be + instantiated unless all of its abstract methods are overridden. + The abstract methods can be called using any of the normal + 'super' call mechanisms. + + Usage: + + class C: + __metaclass__ = ABCMeta + @abstractmethod + def my_abstract_method(self, ...): + ... + """ + funcobj.__isabstractmethod__ = True + return funcobj + + +class abstractproperty(property): + """A decorator indicating abstract properties. + + Requires that the metaclass is ABCMeta or derived from it. A + class that has a metaclass derived from ABCMeta cannot be + instantiated unless all of its abstract properties are overridden. + The abstract properties can be called using any of the normal + 'super' call mechanisms. + + Usage: + + class C: + __metaclass__ = ABCMeta + @abstractproperty + def my_abstract_property(self): + ... + + This defines a read-only property; you can also define a read-write + abstract property using the 'long' form of property declaration: + + class C: + __metaclass__ = ABCMeta + def getx(self): ... + def setx(self, value): ... + x = abstractproperty(getx, setx) + """ + __isabstractmethod__ = True + + +class ABCMeta(type): + + """Metaclass for defining Abstract Base Classes (ABCs). + + Use this metaclass to create an ABC. An ABC can be subclassed + directly, and then acts as a mix-in class. You can also register + unrelated concrete classes (even built-in classes) and unrelated + ABCs as 'virtual subclasses' -- these and their descendants will + be considered subclasses of the registering ABC by the built-in + issubclass() function, but the registering ABC won't show up in + their MRO (Method Resolution Order) nor will method + implementations defined by the registering ABC be callable (not + even via super()). + + """ + + # A global counter that is incremented each time a class is + # registered as a virtual subclass of anything. It forces the + # negative cache to be cleared before its next use. + _abc_invalidation_counter = 0 + + def __new__(mcls, name, bases, namespace): + cls = super(ABCMeta, mcls).__new__(mcls, name, bases, namespace) + # Compute set of abstract method names + abstracts = set(name + for name, value in namespace.items() + if getattr(value, "__isabstractmethod__", False)) + for base in bases: + for name in getattr(base, "__abstractmethods__", set()): + value = getattr(cls, name, None) + if getattr(value, "__isabstractmethod__", False): + abstracts.add(name) + cls.__abstractmethods__ = frozenset(abstracts) + # Set up inheritance registry + cls._abc_registry = WeakSet() + cls._abc_cache = WeakSet() + cls._abc_negative_cache = WeakSet() + cls._abc_negative_cache_version = ABCMeta._abc_invalidation_counter + return cls + + def register(cls, subclass): + """Register a virtual subclass of an ABC.""" + if not isinstance(subclass, (type, types.ClassType)): + raise TypeError("Can only register classes") + if issubclass(subclass, cls): + return # Already a subclass + # Subtle: test for cycles *after* testing for "already a subclass"; + # this means we allow X.register(X) and interpret it as a no-op. + if issubclass(cls, subclass): + # This would create a cycle, which is bad for the algorithm below + raise RuntimeError("Refusing to create an inheritance cycle") + cls._abc_registry.add(subclass) + ABCMeta._abc_invalidation_counter += 1 # Invalidate negative cache + + def _dump_registry(cls, file=None): + """Debug helper to print the ABC registry.""" + print >> file, "Class: %s.%s" % (cls.__module__, cls.__name__) + print >> file, "Inv.counter: %s" % ABCMeta._abc_invalidation_counter + for name in sorted(cls.__dict__.keys()): + if name.startswith("_abc_"): + value = getattr(cls, name) + print >> file, "%s: %r" % (name, value) + + def __instancecheck__(cls, instance): + """Override for isinstance(instance, cls).""" + # Inline the cache checking when it's simple. + subclass = getattr(instance, '__class__', None) + if subclass is not None and subclass in cls._abc_cache: + return True + subtype = type(instance) + # Old-style instances + if subtype is _InstanceType: + subtype = subclass + if subtype is subclass or subclass is None: + if (cls._abc_negative_cache_version == + ABCMeta._abc_invalidation_counter and + subtype in cls._abc_negative_cache): + return False + # Fall back to the subclass check. + return cls.__subclasscheck__(subtype) + return (cls.__subclasscheck__(subclass) or + cls.__subclasscheck__(subtype)) + + def __subclasscheck__(cls, subclass): + """Override for issubclass(subclass, cls).""" + # Check cache + if subclass in cls._abc_cache: + return True + # Check negative cache; may have to invalidate + if cls._abc_negative_cache_version < ABCMeta._abc_invalidation_counter: + # Invalidate the negative cache + cls._abc_negative_cache = WeakSet() + cls._abc_negative_cache_version = ABCMeta._abc_invalidation_counter + elif subclass in cls._abc_negative_cache: + return False + # Check the subclass hook + ok = cls.__subclasshook__(subclass) + if ok is not NotImplemented: + assert isinstance(ok, bool) + if ok: + cls._abc_cache.add(subclass) + else: + cls._abc_negative_cache.add(subclass) + return ok + # Check if it's a direct subclass + if cls in getattr(subclass, '__mro__', ()): + cls._abc_cache.add(subclass) + return True + # Check if it's a subclass of a registered class (recursive) + for rcls in cls._abc_registry: + if issubclass(subclass, rcls): + cls._abc_cache.add(subclass) + return True + # Check if it's a subclass of a subclass (recursive) + for scls in cls.__subclasses__(): + if issubclass(subclass, scls): + cls._abc_cache.add(subclass) + return True + # No dice; update negative cache + cls._abc_negative_cache.add(subclass) + return False diff --git a/python/testData/inspections/PyMethodMayBeStaticInspection/propertyWithAlias.py b/python/testData/inspections/PyMethodMayBeStaticInspection/propertyWithAlias.py new file mode 100644 index 000000000000..b2dfb22ef940 --- /dev/null +++ b/python/testData/inspections/PyMethodMayBeStaticInspection/propertyWithAlias.py @@ -0,0 +1,8 @@ +from abc import abstractproperty as alias, ABCMeta + +class MyAbsBase(object): + __metaclass__ = ABCMeta + + @alias + def count(self): # <- false positive here + return \ No newline at end of file diff --git a/python/testSrc/com/jetbrains/python/inspections/PyMethodMayBeStaticInspectionTest.java b/python/testSrc/com/jetbrains/python/inspections/PyMethodMayBeStaticInspectionTest.java index 931cf9ef78ff..82ee6bde7b44 100644 --- a/python/testSrc/com/jetbrains/python/inspections/PyMethodMayBeStaticInspectionTest.java +++ b/python/testSrc/com/jetbrains/python/inspections/PyMethodMayBeStaticInspectionTest.java @@ -1,7 +1,10 @@ package com.jetbrains.python.inspections; +import com.jetbrains.python.PythonTestUtil; import com.jetbrains.python.fixtures.PyTestCase; +import java.util.Arrays; + /** * User: ktisha */ @@ -56,12 +59,29 @@ public class PyMethodMayBeStaticInspectionTest extends PyTestCase { } public void testAbstractProperty() { - doTest(); + doMultiFileTest("abc.py"); + } + + public void testPropertyWithAlias() { + doMultiFileTest("abc.py"); } private void doTest() { - myFixture.configureByFile("inspections/PyMethodMayBeStaticInspection/" + getTestName(true) + ".py"); + myFixture.configureByFile(getTestName(true) + ".py"); myFixture.enableInspections(PyMethodMayBeStaticInspection.class); myFixture.checkHighlighting(false, false, true); } + + private void doMultiFileTest(String ... files) { + String [] filenames = Arrays.copyOf(files, files.length + 1); + filenames[files.length] = getTestName(true) + ".py"; + myFixture.configureByFiles(filenames); + myFixture.enableInspections(PyMethodMayBeStaticInspection.class); + myFixture.checkHighlighting(false, false, true); + } + + @Override + protected String getTestDataPath() { + return PythonTestUtil.getTestDataPath() + "/inspections/PyMethodMayBeStaticInspection/"; + } }