From kde-commits Wed Nov 30 23:27:07 2016 From: Francis Herne Date: Wed, 30 Nov 2016 23:27:07 +0000 To: kde-commits Subject: [kdev-python] duchain: Make ExpressionVisitor::visitCall() clearer. Message-Id: X-MARC-Message: https://marc.info/?l=kde-commits&m=148054843631111 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 =3D 0; - FunctionType::Ptr unidentifiedFunctionType; - if ( ! v.m_isAlias && v.lastType() && v.lastType()->whichType() =3D=3D= AbstractType::TypeFunction ) { - unidentifiedFunctionType =3D v.lastType().cast(); - } - else if ( ! v.m_isAlias && v.lastType() && v.lastType()->whichType() = =3D=3D AbstractType::TypeStructure ) { - // use __call__ - DUChainReadLocker lock; - auto c =3D v.lastType().cast()->internalContext(top= Context()); - if ( c ) { - auto decls =3D c->findDeclarations(QualifiedIdentifier("__call= __")); - if ( ! decls.isEmpty() ) { - auto decl =3D dynamic_cast(decls.fir= st()); - if ( decl ) { - unidentifiedFunctionType =3D decl->abstractType().cast= (); - } - } + auto declaration =3D Helper::resolveAliasDeclaration(v.lastDeclaration= ().data()); + if ( ! v.isAlias() && v.lastType() ) { + if ( auto functionType =3D v.lastType().cast() ) { + encounter(functionType->returnType()); + return; + } + if ( auto classType =3D v.lastType().cast() ) { + declaration =3D classType->declaration(topContext()); } } - else { - actualDeclaration =3D v.lastDeclaration().data(); - } - = - if ( unidentifiedFunctionType ) { - encounter(unidentifiedFunctionType->returnType()); + if ( ! declaration ) { + encounterUnknown(); return; } - else if ( !actualDeclaration ) { - setConfident(false); - return encounterUnknown(); - } - + ClassDeclaration* classDecl =3D dynamic_cast(declar= ation); DUChainReadLocker lock; - actualDeclaration =3D Helper::resolveAliasDeclaration(actualDeclaratio= n); - ClassDeclaration* classDecl =3D dynamic_cast(actual= Declaration); - auto function =3D Helper::functionForCalled(actualDeclaration); + auto function =3D Helper::functionForCalled(declaration, v.isAlias()); lock.unlock(); = - if ( function.declaration && function.declaration->type(= ) ) { - // 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 =3D classDecl->abstractType(); + decl =3D classDecl; } - else if ( classDecl ) { - return encounter(classDecl->abstractType(), DeclarationPointer(cla= ssDecl)); + else if ( function.declaration && function.declaration->type() ) { + // But do use the return value of normal functions or __call__(). + type =3D function.declaration->type()->returnType(); + decl =3D 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 fun= ction declaration"; + encounterUnknown(); + return; + } + if ( function.declaration ) { + auto docstring =3D function.declaration->comment(); + if ( ! docstring.isEmpty() ) { + // Our documentation data uses special docstrings that overrid= e the return type + // of some functions (including constructors). + type =3D docstringTypeOverride(node, type, docstring); } - return encounterUnknown(); } + encounter(type, DeclarationPointer(decl)); } = -void ExpressionVisitor::checkForDecorators(CallAst* node, FunctionDeclarat= ion* funcDecl, ClassDeclaration* classDecl, bool isConstructor) +AbstractType::Ptr ExpressionVisitor::docstringTypeOverride( + CallAst* node, const AbstractType::Ptr normalType, const QString& docs= tring) { - AbstractType::Ptr type; - Declaration* useDeclaration =3D nullptr; - if ( isConstructor && classDecl ) { - type =3D classDecl->abstractType(); - useDeclaration =3D classDecl; - } - else { - type =3D funcDecl->type()->returnType(); - useDeclaration =3D funcDecl; - } - + auto docstringType =3D normalType; auto listOfTuples =3D [&](AbstractType::Ptr key, AbstractType::Ptr val= ue) { auto newType =3D typeObjectForIntegralType("list"); IndexedContainer::Ptr newContents =3D typeObjectForIntegralType("tuple"); @@ -214,8 +200,7 @@ void ExpressionVisitor::checkForDecorators(CallAst* nod= e, FunctionDeclaration* f baseTypeVisitor.visitNode(static_cast(node->functio= n)->value); if ( auto t =3D baseTypeVisitor.lastType().cast() ) { qCDebug(KDEV_PYTHON_DUCHAIN) << "Found container, using type"; - AbstractType::Ptr newType =3D t->contentType().abstractType(); - encounter(newType, DeclarationPointer(useDeclaration)); + docstringType =3D t->contentType().abstractType(); return true; } return false; @@ -243,8 +228,7 @@ void ExpressionVisitor::checkForDecorators(CallAst* nod= e, FunctionDeclaration* f contentType =3D map->keyType().abstractType(); } newType->addContentType(contentType); - AbstractType::Ptr resultingType =3D newType.cast= (); - encounter(resultingType, DeclarationPointer(useDeclaration)); + docstringType =3D newType.cast(); return true; } return false; @@ -261,8 +245,7 @@ void ExpressionVisitor::checkForDecorators(CallAst* nod= e, FunctionDeclaration* f DUChainWriteLocker lock; auto intType =3D typeObjectForIntegralType("int"); auto enumerated =3D enumeratedTypeVisitor.lastType(); - auto result =3D listOfTuples(intType, Helper::contentOfIterable(en= umerated, topContext())); - encounter(result, DeclarationPointer(useDeclaration)); + docstringType =3D listOfTuples(intType, Helper::contentOfIterable(= enumerated, topContext())); return true; }; = @@ -277,8 +260,7 @@ void ExpressionVisitor::checkForDecorators(CallAst* nod= e, FunctionDeclaration* f DUChainWriteLocker lock; if ( auto t =3D baseTypeVisitor.lastType().cast() ) { qCDebug(KDEV_PYTHON_DUCHAIN) << "Got container:" << t->toStrin= g(); - auto resultingType =3D listOfTuples(t->keyType().abstractType(= ), t->contentType().abstractType()); - encounter(resultingType, DeclarationPointer(useDeclaration)); + docstringType =3D listOfTuples(t->keyType().abstractType(), t-= >contentType().abstractType()); return true; } return false; @@ -297,7 +279,7 @@ void ExpressionVisitor::checkForDecorators(CallAst* nod= e, FunctionDeclaration* f return false; } ListType::Ptr realTarget; - if ( auto target =3D ListType::Ptr::dynamicCast(type) ) { + if ( auto target =3D ListType::Ptr::dynamicCast(normalType) ) { realTarget =3D target; } if ( auto source =3D ListType::Ptr::dynamicCast(v.lastType()) ) { @@ -308,29 +290,26 @@ void ExpressionVisitor::checkForDecorators(CallAst* n= ode, FunctionDeclaration* f auto newType =3D ListType::Ptr::staticCast(AbstractType::Ptr(r= ealTarget->clone())); Q_ASSERT(newType); newType->addContentType(source->contentTyp= e().abstractType()); - encounter(AbstractType::Ptr::staticCast(newType), DeclarationP= ointer(useDeclaration)); + docstringType =3D AbstractType::Ptr::staticCast(newType); return true; } return false; }; = - auto docstring =3D 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 evaluati= on 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, &argu= ments) ) { + continue; + } + // If the hint word appears in the docstring, run the evaluation f= unction. + 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 ty= pe. * - * @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* fu= ncDecl, - Python::ClassDeclaration* classDecl, bool isCo= nstructor); + AbstractType::Ptr docstringTypeOverride(CallAst* node, const AbstractT= ype::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()", Continu= e); + 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): r= eturn iter(Gen2.contents)\n" "for checkme in Gen2(): pa= ss" << "int"; = + QTest::newRow("init_class") << "class Foo:\n" + " def __init__(self): pass\n" + " def __call__(self): return 1.5\n" + "checkme =3D 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 =3D [Foo]\n" + "checkme =3D a[0]()\n" << "Foo= "; QTest::newRow("call_class") << "class Foo:\n" " def __call__(self):\n" " return 0\n" "f =3D Foo()\n" "checkme =3D f()\n" << "int"; + QTest::newRow("call_class_no_decl") << "class Foo:\n" + " def __call__(self): return 1= .5\n" + "a =3D [Foo()]\n" + "checkme =3D a[0]()" << "float"; QTest::newRow("classmethod") << "class Foo:\n" " @classmethod\n" " def foo(cls):\n"