[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