From 8074135fd411450d17eca4b63797fcf09b597e62 Mon Sep 17 00:00:00 2001 From: Semyon Proshev Date: Fri, 22 Jul 2016 17:25:46 +0300 Subject: [PATCH] Attempt to make PyMissingConstructorInspection and PyMissingConstructorTest more readable --- .../PyMissingConstructorInspection.java | 238 ++++++++++-------- .../inspections/PyMissingConstructorTest.java | 54 ++-- 2 files changed, 170 insertions(+), 122 deletions(-) diff --git a/python/src/com/jetbrains/python/inspections/PyMissingConstructorInspection.java b/python/src/com/jetbrains/python/inspections/PyMissingConstructorInspection.java index f27981488049..7502481940ee 100644 --- a/python/src/com/jetbrains/python/inspections/PyMissingConstructorInspection.java +++ b/python/src/com/jetbrains/python/inspections/PyMissingConstructorInspection.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2014 JetBrains s.r.o. + * Copyright 2000-2016 JetBrains s.r.o. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,6 +29,8 @@ import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.Optional; + import static com.jetbrains.python.PyNames.*; /** @@ -37,6 +39,7 @@ import static com.jetbrains.python.PyNames.*; * Inspection to warn if call to super constructor in class is missed */ public class PyMissingConstructorInspection extends PyInspection { + @Nls @NotNull @Override @@ -46,141 +49,180 @@ public class PyMissingConstructorInspection extends PyInspection { @NotNull @Override - public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, - boolean isOnTheFly, - @NotNull LocalInspectionToolSession session) { + public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly, @NotNull LocalInspectionToolSession session) { return new Visitor(holder, session); } private static class Visitor extends PyInspectionVisitor { + public Visitor(@Nullable ProblemsHolder holder, @NotNull LocalInspectionToolSession session) { super(holder, session); } @Override - public void visitPyClass(final PyClass node) { - PsiElement[] superClasses = node.getSuperClassExpressions(); - if (superClasses.length == 0 || (superClasses.length == 1 && OBJECT.equals(superClasses[0].getText()))) - return; + public void visitPyClass(@NotNull PyClass node) { + final PsiElement[] superClasses = node.getSuperClassExpressions(); - if (!superHasConstructor(node)) return; - PyFunction initMethod = node.findMethodByName(INIT, false, null); - if (initMethod != null) { - if (isExceptionClass(node, myTypeEvalContext) || hasConstructorCall(node, initMethod)) { - return; - } - if (superClasses.length == 1 || node.isNewStyleClass(null)) - registerProblem(initMethod.getNameIdentifier(), PyBundle.message("INSP.missing.super.constructor.message"), - new AddCallSuperQuickFix()); - else - registerProblem(initMethod.getNameIdentifier(), PyBundle.message("INSP.missing.super.constructor.message")); + if (superClasses.length == 0 || + superClasses.length == 1 && OBJECT.equals(superClasses[0].getText()) || + !superHasConstructor(node, myTypeEvalContext)) { + return; + } + + final PyFunction initMethod = node.findMethodByName(INIT, false, myTypeEvalContext); + + if (initMethod == null || isExceptionClass(node, myTypeEvalContext) || hasConstructorCall(node, initMethod, myTypeEvalContext)) { + return; + } + + if (superClasses.length == 1 || node.isNewStyleClass(myTypeEvalContext)) { + registerProblem(initMethod.getNameIdentifier(), PyBundle.message("INSP.missing.super.constructor.message"), + new AddCallSuperQuickFix()); + } + else { + registerProblem(initMethod.getNameIdentifier(), PyBundle.message("INSP.missing.super.constructor.message")); } } - private boolean superHasConstructor(@NotNull PyClass cls) { + private static boolean superHasConstructor(@NotNull PyClass cls, @NotNull TypeEvalContext context) { final String className = cls.getName(); - for (PyClass c : cls.getAncestorClasses(myTypeEvalContext)) { - final String name = c.getName(); - if (!PyUtil.isObjectClass(c) && !Comparing.equal(className, name) && c.findMethodByName(INIT, false, null) != null) { + + for (PyClass baseClass : cls.getAncestorClasses(context)) { + if (!PyUtil.isObjectClass(baseClass) && + !Comparing.equal(className, baseClass.getName()) && + baseClass.findMethodByName(INIT, false, context) != null) { return true; } } + return false; } - private boolean isExceptionClass(@NotNull PyClass cls, @NotNull TypeEvalContext context) { + private static boolean isExceptionClass(@NotNull PyClass cls, @NotNull TypeEvalContext context) { if (PyBroadExceptionInspection.equalsException(cls, context)) { return true; } - for (PyClass baseClass : cls.getAncestorClasses(myTypeEvalContext)) { - if (PyBroadExceptionInspection.equalsException(baseClass, context)) { - return true; - } - } - return false; + + return cls.getAncestorClasses(context) + .stream() + .filter(baseClass -> PyBroadExceptionInspection.equalsException(baseClass, context)) + .findAny() + .isPresent(); } - private boolean hasConstructorCall(PyClass node, PyFunction initMethod) { - PyStatementList statementList = initMethod.getStatementList(); - CallVisitor visitor = new CallVisitor(node); - if (statementList != null) { - statementList.accept(visitor); - return visitor.myHasConstructorCall; - } - return false; + private static boolean hasConstructorCall(@NotNull PyClass cls, @NotNull PyFunction initMethod, @NotNull TypeEvalContext context) { + final CallVisitor visitor = new CallVisitor(cls, context); + initMethod.getStatementList().accept(visitor); + return visitor.myHasConstructorCall; } - private class CallVisitor extends PyRecursiveElementVisitor { + private static class CallVisitor extends PyRecursiveElementVisitor { + + @NotNull + private final PyClass myClass; + + @NotNull + private final TypeEvalContext myContext; + private boolean myHasConstructorCall = false; - private PyClass myClass; - CallVisitor(PyClass node) { - myClass = node; + + public CallVisitor(@NotNull PyClass cls, @NotNull TypeEvalContext context) { + myClass = cls; + myContext = context; } @Override - public void visitPyCallExpression(PyCallExpression node) { - if (isConstructorCall(node, myClass)) + public void visitPyCallExpression(@NotNull PyCallExpression node) { + if (isConstructorCall(node, myClass, myContext)) { myHasConstructorCall = true; - } - - private boolean isConstructorCall(PyCallExpression expression, PyClass cl) { - PyExpression callee = expression.getCallee(); - if (callee instanceof PyQualifiedExpression) { - PyExpression qualifier = ((PyQualifiedExpression)callee).getQualifier(); - if (qualifier != null) { - String tmp = ""; - if (qualifier instanceof PyCallExpression) { - PyExpression innerCallee = ((PyCallExpression)qualifier).getCallee(); - if (innerCallee != null) { - tmp = innerCallee.getName(); - } - if (SUPER.equals(tmp) && (INIT.equals(callee.getName()))) { - PyExpression[] args = ((PyCallExpression)qualifier).getArguments(); - if (args.length > 0) { - String firstArg = args[0].getText(); - final String qualifiedName = cl.getQualifiedName(); - if (firstArg.equals(cl.getName()) || firstArg.equals(CANONICAL_SELF+"."+ __CLASS__) || - (qualifiedName != null && qualifiedName.endsWith(firstArg)) || - (LanguageLevel.forElement(cl).isAtLeast(LanguageLevel.PYTHON30) && firstArg.equals(__CLASS__))) - return true; - for (PyClass s : cl.getAncestorClasses(myTypeEvalContext)) { - if (firstArg.equals(s.getName())) - return true; - } - } - else - return true; - } - } - if (INIT.equals(callee.getName())) { - return isSuperClassCall(cl, qualifier); - } - } } - return false; } - private boolean isSuperClassCall(PyClass cl, PyExpression qualifier) { - PsiElement callingClass = null; - if (qualifier instanceof PyCallExpression) { - PyExpression innerCallee = ((PyCallExpression)qualifier).getCallee(); - if (innerCallee != null) { - PsiReference ref = innerCallee.getReference(); - if (ref != null) - callingClass = ref.resolve(); - } + private static boolean isConstructorCall(@NotNull PyCallExpression call, @NotNull PyClass cls, @NotNull TypeEvalContext context) { + final PyExpression callee = call.getCallee(); + + if (callee == null || !INIT.equals(callee.getName())) { + return false; + } + + final PyExpression calleeQualifier = Optional + .of(callee) + .filter(PyQualifiedExpression.class::isInstance) + .map(PyQualifiedExpression.class::cast) + .map(PyQualifiedExpression::getQualifier) + .orElse(null); + + return calleeQualifier != null && (isSuperCall(calleeQualifier, cls, context) || isSuperClassCall(calleeQualifier, cls, context)); + } + + private static boolean isSuperCall(@NotNull PyExpression calleeQualifier, + @NotNull PyClass cls, + @NotNull TypeEvalContext context) { + final String prevCalleeName = Optional + .of(calleeQualifier) + .filter(PyCallExpression.class::isInstance) + .map(PyCallExpression.class::cast) + .map(PyCallExpression::getCallee) + .map(PyExpression::getName) + .orElse(null); + + if (!SUPER.equals(prevCalleeName)) { + return false; + } + + final PyExpression[] args = ((PyCallExpression)calleeQualifier).getArguments(); + + if (args.length == 0) { + return true; + } + + final String firstArg = args[0].getText(); + final String classQName = cls.getQualifiedName(); + + if (firstArg.equals(cls.getName()) || + firstArg.equals(CANONICAL_SELF + "." + __CLASS__) || + classQName != null && classQName.endsWith(firstArg) || + firstArg.equals(__CLASS__) && LanguageLevel.forElement(cls).isAtLeast(LanguageLevel.PYTHON30)) { + return true; + } + + return cls.getAncestorClasses(context) + .stream() + .map(PyClass::getName) + .filter(firstArg::equals) + .findAny() + .isPresent(); + } + + private static boolean isSuperClassCall(@NotNull PyExpression calleeQualifier, + @NotNull PyClass cls, + @NotNull TypeEvalContext context) { + final PsiElement callingClass = resolveCallingClass(calleeQualifier); + + return callingClass != null && + cls.getAncestorClasses(context) + .stream() + .filter(callingClass::equals) + .findAny() + .isPresent(); + } + + @Nullable + private static PsiElement resolveCallingClass(@NotNull PyExpression calleeQualifier) { + if (calleeQualifier instanceof PyCallExpression) { + return Optional + .of((PyCallExpression)calleeQualifier) + .map(PyCallExpression::getCallee) + .map(PyExpression::getReference) + .map(PsiReference::resolve) + .orElse(null); } else { - PsiReference ref = qualifier.getReference(); - if (ref != null) - callingClass = ref.resolve(); + return Optional + .ofNullable(calleeQualifier.getReference()) + .map(PsiReference::resolve) + .orElse(null); } - for (PyClass s : cl.getAncestorClasses(myTypeEvalContext)) { - if (s.equals(callingClass)) { - return true; - } - } - return false; } } } diff --git a/python/testSrc/com/jetbrains/python/inspections/PyMissingConstructorTest.java b/python/testSrc/com/jetbrains/python/inspections/PyMissingConstructorTest.java index cc9fe136a631..468437e305a1 100644 --- a/python/testSrc/com/jetbrains/python/inspections/PyMissingConstructorTest.java +++ b/python/testSrc/com/jetbrains/python/inspections/PyMissingConstructorTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2013 JetBrains s.r.o. + * Copyright 2000-2016 JetBrains s.r.o. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,47 +23,53 @@ import com.jetbrains.python.psi.LanguageLevel; */ public class PyMissingConstructorTest extends PyTestCase { private static final String TEST_DIRECTORY = "inspections/PyMissingConstructorInspection/"; - + public void testBasic() { doTest(); } - - public void testQualifiedName() { // PY-3278 - doTest(); - } - - public void testNoConstructor() { // PY-3238 - doTest(); - } - - public void testDeepInheritance() { // PY-3313 - doTest(); - } - - public void testInheritFromSelf() { // PY-3395 + + // PY-3278 + public void testQualifiedName() { doTest(); } - public void testExplicitDunderClass() { // PY-4038 + // PY-3238 + public void testNoConstructor() { doTest(); } - public void testImplicitDunderClass() { // PY-20038 - setLanguageLevel(LanguageLevel.PYTHON30); + // PY-3313 + public void testDeepInheritance() { doTest(); } - public void testException() { // PY-7176 + // PY-3395 + public void testInheritFromSelf() { doTest(); } - public void testInnerClass() { //PY-7699 + // PY-4038 + public void testExplicitDunderClass() { doTest(); } - + + // PY-20038 + public void testImplicitDunderClass() { + runWithLanguageLevel(LanguageLevel.PYTHON30, this::doTest); + } + + // PY-7176 + public void testException() { + doTest(); + } + + // PY-7699 + public void testInnerClass() { + doTest(); + } + public void testPy3k() { - setLanguageLevel(LanguageLevel.PYTHON30); - doTest(); + runWithLanguageLevel(LanguageLevel.PYTHON30, this::doTest); } private void doTest() {