[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [kdev-python] duchain: Make ExpressionVisitor::visitCall() clearer.
From:       Francis Herne <mail () flherne ! uk>
Date:       2016-11-30 23:27:07
Message-ID: E1cCEHL-0005EE-Ky () code ! kde ! org
[Download RAW message or body]

Git commit b56caec83b465d30c0e25ce8dc7e1dc61fdb790e by Francis Herne.
Committed on 30/11/2016 at 23:23.
Pushed by flherne into branch 'master'.

Make ExpressionVisitor::visitCall() clearer.

Rename `checkForDecorators()` to `docstringTypeOverride()`,
 because it doesn't do anything involving decorators. (!)

Move the code choosing between class and function types to `visitCall()`,
 because it's nothing to do with docstring contents and made the
 different code paths very hard to follow.

Make it return the type it produces, rather than calling `encounter()`
 itself, for clarity.

Use the new `isAlias` argument of `functionForCalled` instead of
 some code to do the same thing.

Test `init_class_no_decl` is (still) XFAIL, we can't fix
 that with the current system for class types.

Test `call_class_no_decl` worked before, but I broke that behaviour in
 an earlier version of this patch and realised there wasn't a test.

Differential Revision: https://phabricator.kde.org/D3524

M  +54   -75   duchain/expressionvisitor.cpp
M  +6    -7    duchain/expressionvisitor.h
M  +14   -0    duchain/tests/pyduchaintest.cpp

https://commits.kde.org/kdev-python/b56caec83b465d30c0e25ce8dc7e1dc61fdb790e

diff --git a/duchain/expressionvisitor.cpp b/duchain/expressionvisitor.cpp
index ca3d4c8..7a22ae4 100644
--- a/duchain/expressionvisitor.cpp
+++ b/duchain/expressionvisitor.cpp
@@ -115,75 +115,61 @@ void ExpressionVisitor::visitCall(CallAst* node)
     foreach ( ExpressionAst* c, node->arguments ) {
         AstDefaultVisitor::visitNode(c);
     }
-    
     ExpressionVisitor v(this);
     v.visitNode(node->function);
-    Declaration* actualDeclaration = 0;
-    FunctionType::Ptr unidentifiedFunctionType;
-    if ( ! v.m_isAlias && v.lastType() && v.lastType()->whichType() == \
                AbstractType::TypeFunction ) {
-        unidentifiedFunctionType = v.lastType().cast<FunctionType>();
-    }
-    else if ( ! v.m_isAlias && v.lastType() && v.lastType()->whichType() == \
                AbstractType::TypeStructure ) {
-        // use __call__
-        DUChainReadLocker lock;
-        auto c = v.lastType().cast<StructureType>()->internalContext(topContext());
-        if ( c ) {
-            auto decls = c->findDeclarations(QualifiedIdentifier("__call__"));
-            if ( ! decls.isEmpty() ) {
-                auto decl = dynamic_cast<FunctionDeclaration*>(decls.first());
-                if ( decl ) {
-                    unidentifiedFunctionType = \
                decl->abstractType().cast<FunctionType>();
-                }
-            }
+    auto declaration = Helper::resolveAliasDeclaration(v.lastDeclaration().data());
+    if ( ! v.isAlias() && v.lastType() ) {
+        if ( auto functionType = v.lastType().cast<FunctionType>() ) {
+            encounter(functionType->returnType());
+            return;
+        }
+        if ( auto classType = v.lastType().cast<StructureType>() ) {
+            declaration = classType->declaration(topContext());
         }
     }
-    else {
-        actualDeclaration = v.lastDeclaration().data();
-    }
-    
-    if ( unidentifiedFunctionType ) {
-        encounter(unidentifiedFunctionType->returnType());
+    if ( ! declaration ) {
+        encounterUnknown();
         return;
     }
-    else if ( !actualDeclaration ) {
-        setConfident(false);
-        return encounterUnknown();
-    }
-
+    ClassDeclaration* classDecl = dynamic_cast<ClassDeclaration*>(declaration);
     DUChainReadLocker lock;
-    actualDeclaration = Helper::resolveAliasDeclaration(actualDeclaration);
-    ClassDeclaration* classDecl = \
                dynamic_cast<ClassDeclaration*>(actualDeclaration);
-    auto function = Helper::functionForCalled(actualDeclaration);
+    auto function = Helper::functionForCalled(declaration, v.isAlias());
     lock.unlock();
 
-    if ( function.declaration && function.declaration->type<FunctionType>() ) {
-        // try to deduce type from a decorator
-        checkForDecorators(node, function.declaration, classDecl, \
function.isConstructor); +    AbstractType::Ptr type;
+    Declaration* decl;
+
+    if ( function.isConstructor && classDecl ) {
+        // Don't use return type from constructor.
+        // It's wrong for builtins, or classes without their own __init__ methods().
+        type = classDecl->abstractType();
+        decl = classDecl;
     }
-    else if ( classDecl ) {
-        return encounter(classDecl->abstractType(), DeclarationPointer(classDecl));
+    else if ( function.declaration && function.declaration->type<FunctionType>() ) {
+        // But do use the return value of normal functions or __call__().
+        type = function.declaration->type<FunctionType>()->returnType();
+        decl = function.declaration;
     }
     else {
-        if ( actualDeclaration ) {
-            qCDebug(KDEV_PYTHON_DUCHAIN) << "Declaraton is not a class or function \
declaration"; +        qCDebug(KDEV_PYTHON_DUCHAIN) << "Declaration is not a class or \
function declaration"; +        encounterUnknown();
+        return;
+    }
+    if ( function.declaration ) {
+        auto docstring = function.declaration->comment();
+        if ( ! docstring.isEmpty() ) {
+            // Our documentation data uses special docstrings that override the \
return type +            //  of some functions (including constructors).
+            type = docstringTypeOverride(node, type, docstring);
         }
-        return encounterUnknown();
     }
+    encounter(type, DeclarationPointer(decl));
 }
 
-void ExpressionVisitor::checkForDecorators(CallAst* node, FunctionDeclaration* \
funcDecl, ClassDeclaration* classDecl, bool isConstructor) +AbstractType::Ptr \
ExpressionVisitor::docstringTypeOverride( +    CallAst* node, const AbstractType::Ptr \
normalType, const QString& docstring)  {
-    AbstractType::Ptr type;
-    Declaration* useDeclaration = nullptr;
-    if ( isConstructor && classDecl ) {
-        type = classDecl->abstractType();
-        useDeclaration = classDecl;
-    }
-    else {
-        type = funcDecl->type<FunctionType>()->returnType();
-        useDeclaration = funcDecl;
-    }
-
+    auto docstringType = normalType;
     auto listOfTuples = [&](AbstractType::Ptr key, AbstractType::Ptr value) {
         auto newType = typeObjectForIntegralType<ListType>("list");
         IndexedContainer::Ptr newContents = \
typeObjectForIntegralType<IndexedContainer>("tuple"); @@ -214,8 +200,7 @@ void \
                ExpressionVisitor::checkForDecorators(CallAst* node, \
                FunctionDeclaration* f
         baseTypeVisitor.visitNode(static_cast<AttributeAst*>(node->function)->value);
  if ( auto t = baseTypeVisitor.lastType().cast<ListType>() ) {
             qCDebug(KDEV_PYTHON_DUCHAIN) << "Found container, using type";
-            AbstractType::Ptr newType = t->contentType().abstractType();
-            encounter(newType, DeclarationPointer(useDeclaration));
+            docstringType = t->contentType().abstractType();
             return true;
         }
         return false;
@@ -243,8 +228,7 @@ void ExpressionVisitor::checkForDecorators(CallAst* node, \
FunctionDeclaration* f  contentType = map->keyType().abstractType();
             }
             newType->addContentType<Python::UnsureType>(contentType);
-            AbstractType::Ptr resultingType = newType.cast<AbstractType>();
-            encounter(resultingType, DeclarationPointer(useDeclaration));
+            docstringType = newType.cast<AbstractType>();
             return true;
         }
         return false;
@@ -261,8 +245,7 @@ void ExpressionVisitor::checkForDecorators(CallAst* node, \
FunctionDeclaration* f  DUChainWriteLocker lock;
         auto intType = typeObjectForIntegralType<AbstractType>("int");
         auto enumerated = enumeratedTypeVisitor.lastType();
-        auto result = listOfTuples(intType, Helper::contentOfIterable(enumerated, \
                topContext()));
-        encounter(result, DeclarationPointer(useDeclaration));
+        docstringType = listOfTuples(intType, Helper::contentOfIterable(enumerated, \
topContext()));  return true;
     };
 
@@ -277,8 +260,7 @@ void ExpressionVisitor::checkForDecorators(CallAst* node, \
FunctionDeclaration* f  DUChainWriteLocker lock;
         if ( auto t = baseTypeVisitor.lastType().cast<MapType>() ) {
             qCDebug(KDEV_PYTHON_DUCHAIN) << "Got container:" << t->toString();
-            auto resultingType = listOfTuples(t->keyType().abstractType(), \
                t->contentType().abstractType());
-            encounter(resultingType, DeclarationPointer(useDeclaration));
+            docstringType = listOfTuples(t->keyType().abstractType(), \
t->contentType().abstractType());  return true;
         }
         return false;
@@ -297,7 +279,7 @@ void ExpressionVisitor::checkForDecorators(CallAst* node, \
FunctionDeclaration* f  return false;
         }
         ListType::Ptr realTarget;
-        if ( auto target = ListType::Ptr::dynamicCast(type) ) {
+        if ( auto target = ListType::Ptr::dynamicCast(normalType) ) {
             realTarget = target;
         }
         if ( auto source = ListType::Ptr::dynamicCast(v.lastType()) ) {
@@ -308,29 +290,26 @@ void ExpressionVisitor::checkForDecorators(CallAst* node, \
                FunctionDeclaration* f
             auto newType = \
ListType::Ptr::staticCast(AbstractType::Ptr(realTarget->clone()));  \
                Q_ASSERT(newType);
             newType->addContentType<Python::UnsureType>(source->contentType().abstractType());
                
-            encounter(AbstractType::Ptr::staticCast(newType), \
DeclarationPointer(useDeclaration)); +            docstringType = \
AbstractType::Ptr::staticCast(newType);  return true;
         }
         return false;
     };
 
-    auto docstring = funcDecl->comment();
-    if ( ! docstring.isEmpty() ) {
-        foreach ( const QString& currentHint, knownDecoratorHints.keys() ) {
-            QStringList arguments;
-            if ( ! Helper::docstringContainsHint(docstring, currentHint, &arguments) \
                ) {
-                continue;
-            }
-            // If the hint word appears in the docstring, run the evaluation \
                function.
-            if ( knownDecoratorHints[currentHint](arguments, currentHint) ) {
-                // We indeed found something, so we're done.
-                return;
-            }
+    foreach ( const QString& currentHint, knownDecoratorHints.keys() ) {
+        QStringList arguments;
+        if ( ! Helper::docstringContainsHint(docstring, currentHint, &arguments) ) {
+            continue;
+        }
+        // If the hint word appears in the docstring, run the evaluation function.
+        if ( knownDecoratorHints[currentHint](arguments, currentHint) ) {
+            // We indeed found something, so we're done.
+            return docstringType;
         }
     }
 
     // if none of the above decorator-finding methods worked, just use the ordinary \
                return type.
-    return encounter(type, DeclarationPointer(useDeclaration));
+    return docstringType;
 }
 
 void ExpressionVisitor::visitSubscript(SubscriptAst* node)
diff --git a/duchain/expressionvisitor.h b/duchain/expressionvisitor.h
index cb3024b..31585c4 100644
--- a/duchain/expressionvisitor.h
+++ b/duchain/expressionvisitor.h
@@ -80,15 +80,14 @@ public:
     virtual void visitNameConstant(NameConstantAst* node);
 
     /**
-     * @brief Checks the decorators of the given function declaration.
+     * @brief Checks for magic docstrings that override a call's return type.
      *
-     * @param node The node to visit
-     * @param funcDecl The call's function declaration, if any
-     * @param classDecl The call's class declaration, if any
-     * @param isConstructor whether a constructor is being called
+     * @param node The node to visit.
+     * @param normalType The return type as determined without docstrings.
+     * @param docstring Docstring of the function.
      */
-    void checkForDecorators(CallAst* node, Python::FunctionDeclaration* funcDecl,
-                            Python::ClassDeclaration* classDecl, bool \
isConstructor); +    AbstractType::Ptr docstringTypeOverride(CallAst* node, const \
AbstractType::Ptr normalType, +                                              const \
QString& docstring);  
     bool isAlias() const {
         return m_isAlias;
diff --git a/duchain/tests/pyduchaintest.cpp b/duchain/tests/pyduchaintest.cpp
index c4d0548..8a2d360 100644
--- a/duchain/tests/pyduchaintest.cpp
+++ b/duchain/tests/pyduchaintest.cpp
@@ -795,6 +795,7 @@ void PyDUChainTest::testTypes()
     visitor->visitCode(m_ast.data());
     QEXPECT_FAIL("lambda", "not implemented: aliasing lambdas", Continue);
     QEXPECT_FAIL("return_builtin_iterator", "fake builtin iter()", Continue);
+    QEXPECT_FAIL("init_class_no_decl", "aliasing info lost", Continue);
     QCOMPARE(visitor->found, true);
 }
 
@@ -963,11 +964,24 @@ void PyDUChainTest::testTypes_data()
                                                 "    def __iter__(self): return \
                iter(Gen2.contents)\n"
                                                 "for checkme in Gen2(): pass" << \
"int";  
+    QTest::newRow("init_class") <<  "class Foo:\n"
+                                    "  def __init__(self): pass\n"
+                                    "  def __call__(self): return 1.5\n"
+                                    "checkme = Foo()\n" << "Foo";
+    QTest::newRow("init_class_no_decl") <<  "class Foo:\n"
+                                            "  def __init__(self): pass\n"
+                                            "  def __call__(self): return 1.5\n"
+                                            "a = [Foo]\n"
+                                            "checkme = a[0]()\n" << "Foo";
     QTest::newRow("call_class") << "class Foo:\n"
                                     "    def __call__(self):\n"
                                     "         return 0\n"
                                     "f = Foo()\n"
                                     "checkme = f()\n" << "int";
+    QTest::newRow("call_class_no_decl") << "class Foo:\n"
+                                           "  def __call__(self): return 1.5\n"
+                                           "a = [Foo()]\n"
+                                           "checkme = a[0]()" << "float";
     QTest::newRow("classmethod") << "class Foo:\n"
                                     "    @classmethod\n"
                                     "    def foo(cls):\n"


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic