diff --git a/duchain/declarationbuilder.h b/duchain/declarationbuilder.h --- a/duchain/declarationbuilder.h +++ b/duchain/declarationbuilder.h @@ -117,9 +117,6 @@ template T* visitVariableDeclaration(Identifier* node, Ast* originalAst = 0, Declaration* previous=nullptr, AbstractType::Ptr type = AbstractType::Ptr(), VisitVariableFlags flags=NoVisitVariableFlags); - template T* visitVariableDeclaration(Identifier* node, RangeInRevision range, - AbstractType::Ptr type = AbstractType::Ptr(), - VisitVariableFlags flags=NoVisitVariableFlags); protected: /** @@ -149,37 +146,14 @@ /// Helper for the above void adjustExpressionsForTypecheck(ExpressionAst* adjust, ExpressionAst* from, bool useUnsure); - /** - * @brief Get a list of the left-hand-side expressions of an assignment. - * - * @param targets the AST nodes representing the left-hand side of the assignment - * - * Example: a, (b, c), _ = 3, 5, 7, 9 -> this function returns a, b, c, d - */ - QList targetsOfAssignment(QList targets) const; - - /// Represents a single source type in a tuple assignment. struct SourceType { AbstractType::Ptr type; DeclarationPointer declaration; bool isAlias; }; /** - * @brief Attempts to determine the unpacked type(s) of the right-hand side of an assignment. - * - * @param source The source type on the right-hand side of the assignment - * @param fillWhenLengthMissing If non-zero and the object in @p items has an indefinite length but - * a definite content type, return this many copies of that type - * - * Example: a, b = [1, 2, 3] and called with @p fillWhenLengthMissing = 2 -> returns int, int - * a, b = "Foo", 3.5 (@p fillWhenLengthMissing ignored here because the right-hand side - * has a definite amount of items) -> returns str, float - */ - QVector unpackAssignmentSource(const SourceType& source, int fillWhenLengthMissing) const; - - /** * @brief Handle a variable assignment to @p name and give it the type @p element. */ void assignToName(NameAst* name, const SourceType& element); @@ -202,6 +176,7 @@ /** * @brief Handle assignment to a target @p target with rhs type @p element. */ + void assignToUnknown(ExpressionAst* target, const AbstractType::Ptr type); void assignToUnknown(ExpressionAst* target, const SourceType& element); /** diff --git a/duchain/declarationbuilder.cpp b/duchain/declarationbuilder.cpp --- a/duchain/declarationbuilder.cpp +++ b/duchain/declarationbuilder.cpp @@ -1,7 +1,8 @@ /***************************************************************************** * Copyright (c) 2007 Piyush verma * * Copyright 2007 Andreas Pakulat * - * Copyright 2010-2013 Sven Brauch * + * Copyright 2010-2016 Sven Brauch * + * Copyright 2016 Francis Herne * * * * This program is free software; you can redistribute it and/or * * modify it under the terms of the GNU General Public License as * @@ -165,16 +166,6 @@ } } -template T* DeclarationBuilder::visitVariableDeclaration(Identifier* node, RangeInRevision range, - AbstractType::Ptr type, VisitVariableFlags flags) -{ - Ast pseudo; - pseudo.startLine = range.start.line; pseudo.startCol = range.start.column; - pseudo.endLine = range.end.line; pseudo.endCol = range.end.column; - T* result = visitVariableDeclaration(node, &pseudo, nullptr, type, flags); - return result; -} - QList< Declaration* > DeclarationBuilder::existingDeclarationsForNode(Identifier* node) { QList existingDeclarations = currentContext()->findDeclarations( @@ -395,75 +386,10 @@ void DeclarationBuilder::visitFor(ForAst* node) { - ExpressionVisitor v(currentContext()); - v.visitNode(node->iterator); - auto possibleIterators = Helper::filterType(v.lastType(), - [](AbstractType::Ptr type) { - auto container = type.cast(); - return container && container->contentType(); - } - ); - if ( node->target->astType == Ast::NameAstType ) { - // In case the iterator variable is a Name ("for x in range(3)"), just create a declaration for it. - // The following code tries to figure out the type of "x" from the object that is being iterated over. - auto iteratorType = Helper::foldTypes(possibleIterators, - [](const ListType::Ptr& p) { - return p->contentType().abstractType(); - } - ); - // otherwise, no list type whatsoever was available for the iterator list, so just display "mixed". - - // Create the variable declaration for the iterator variable with the type that has been determined. - visitVariableDeclaration(node->target, 0, iteratorType); - } - else if ( node->target->astType == Ast::TupleAstType ) { - // If the target is a tuple ("for x, y, z in ..."), multiple variables must be declared. - // For now, types of those variables will only be determined if the iterator is a list of tuples. - QList targetElements = targetsOfAssignment(QList() << node->target); - int targetElementsCount = targetElements.count(); - QList gatherFromTuples; - for ( auto container : possibleIterators ) { - AbstractType::Ptr contentType = container->contentType().abstractType(); - gatherFromTuples = Helper::filterType(contentType, - // find all IndexedContainer entries which have the right number of entries - [targetElementsCount](AbstractType::Ptr type) { - IndexedContainer::Ptr indexed = type.cast(); - return indexed && indexed->typesCount() == targetElementsCount; - } - ); - } - - // Now, iterate over all possible tuples, and extract their types - int i = 0; - QList targetTypes; - foreach ( IndexedContainer::Ptr tuple, gatherFromTuples ) { - for ( int j = 0; j < tuple->typesCount(); j++ ) { - if ( i == 0 ) { - targetTypes.append(tuple->typeAt(j).abstractType()); - } - else { - targetTypes[j] = Helper::mergeTypes(targetTypes[j], tuple->typeAt(j).abstractType()); - } - } - i++; - } - - short atElement = 0; - bool haveTypeInformation = ! targetTypes.isEmpty(); - Q_ASSERT( ! haveTypeInformation || targetTypes.length() == targetElementsCount ); - foreach ( ExpressionAst* tupleMember, targetElements ) { - if ( tupleMember->astType == Ast::NameAstType ) { - AbstractType::Ptr newType; - if ( haveTypeInformation ) { - newType = targetTypes.at(atElement); - } - else { - newType = AbstractType::Ptr(new IntegralType(IntegralType::TypeMixed)); - } - visitVariableDeclaration(tupleMember, 0, newType); - } - ++atElement; - } + if ( node->iterator ) { + ExpressionVisitor v(currentContext()); + v.visitNode(node->iterator); + assignToUnknown(node->target, Helper::contentOfIterable(v.lastType())); } Python::ContextBuilder::visitFor(node); } @@ -548,41 +474,31 @@ } } +void spoofNodePosition(Ast* node, int column) { + // Ridiculous hack, see next comment. + node->startCol = node-> endCol = column; + if (node->astType == Ast::TupleAstType) { + // Recursion to bodge all declarations, e.g. + // [x + y * z for x, (y, z) in foo] + foreach(auto elem, static_cast(node)->elements) { + spoofNodePosition(elem, column); + } + } +} + void DeclarationBuilder::visitComprehension(ComprehensionAst* node) { Python::AstDefaultVisitor::visitComprehension(node); // make the declaration zero chars long; it must appear at the beginning of the context, // because it is actually used *before* its real declaration: [foo for foo in bar] // The DUChain doesn't like this, so for now, the declaration is at the opening bracket, // and both other occurrences are uses of that declaration. // TODO add a special case to the usebuilder to display the second occurrence as a declaration - RangeInRevision declarationRange(currentContext()->range().start, currentContext()->range().start); - declarationRange.end.column -= 1; - declarationRange.start.column -= 1; - - AbstractType::Ptr targetType(new IntegralType(IntegralType::TypeMixed)); - if ( node->iterator ) { - // try to find the type of the object being iterated over, for guessing the - // type of the iterator variable - ExpressionVisitor v(currentContext()); - v.visitNode(node->iterator); - if ( auto container = ListType::Ptr::dynamicCast(v.lastType()) ) { - targetType = container->contentType().abstractType(); - } - } - - // create variable declarations for the iterator variable(s) - if ( node->target->astType == Ast::NameAstType ) { - visitVariableDeclaration(static_cast(node->target)->identifier, declarationRange, targetType); - } - if ( node->target->astType == Ast::TupleAstType ) { - foreach ( ExpressionAst* tupleElt, static_cast(node->target)->elements ) { - if ( tupleElt->astType == Ast::NameAstType ) { - NameAst* n = static_cast(tupleElt); - visitVariableDeclaration(n->identifier, declarationRange); - } - } - } + spoofNodePosition(node->target, currentContext()->range().start.column - 1); + + ExpressionVisitor v(currentContext()); + v.visitNode(node->iterator); + assignToUnknown(node->target, Helper::contentOfIterable(v.lastType())); } void DeclarationBuilder::visitImport(ImportAst* node) @@ -1172,56 +1088,6 @@ addArgumentTypeHints(node, functionVisitor.lastDeclaration()); } -QList DeclarationBuilder::targetsOfAssignment(QList targets) const -{ - QList lhsExpressions; - foreach ( ExpressionAst* target, targets ) { - if ( target->astType == Ast::TupleAstType ) { - TupleAst* tuple = static_cast(target); - foreach ( ExpressionAst* ast, tuple->elements ) { - // eventually recursive call, to handle e.g. a, (b, c) = 1, 2, 3 correctly - if ( ast->astType == Ast::TupleAstType ) { - lhsExpressions << targetsOfAssignment(QList() << ast); - } - else { - // shortcut - lhsExpressions << ast; - } - } - } - else { - lhsExpressions << target; - } - } - return lhsExpressions; -} - -QVector - DeclarationBuilder::unpackAssignmentSource(const SourceType& source, - int fillWhenLengthMissing) const -{ - QVector sources; - if ( const IndexedContainer::Ptr container = source.type.cast() ) { - // RHS is a tuple or similar, unpack that. - for (int i = 0; i < container->typesCount(); ++i) { - sources << SourceType{ - container->typeAt(i).abstractType(), - DeclarationPointer(), - false - }; - } - } - else if ( auto container = ListType::Ptr::dynamicCast(source.type) ) { - // RHS is a list or similar, can't tell contents apart. - // Return content type * requested length. - AbstractType::Ptr content = container->contentType().abstractType(); - for ( ; fillWhenLengthMissing != 0; fillWhenLengthMissing-- ) { - sources << SourceType{ content, DeclarationPointer(), false }; - } - } - return sources; -} - void DeclarationBuilder::assignToName(NameAst* target, const DeclarationBuilder::SourceType& element) { if ( element.isAlias ) { @@ -1353,44 +1219,80 @@ } } -void DeclarationBuilder::assignToTuple(TupleAst* tuple, const DeclarationBuilder::SourceType& element) { - auto sources = unpackAssignmentSource(element, tuple->elements.length()); - int ii = 0; - bool foundStarred = false; - foreach ( ExpressionAst* target, tuple->elements ) { - // Fallback, if we ran out of known types. - auto source = SourceType{ - AbstractType::Ptr(new IntegralType(IntegralType::TypeMixed)), - DeclarationPointer(), - false - }; +void tryUnpackType(AbstractType::Ptr sourceType, QVector& outTypes, int starred) { + // Helper for assignToTuple() below. + // If sourceType is a container that can be unpacked into outTypes, do so. + if ( const auto indexed = sourceType.cast() ) { + int spare = indexed->typesCount() - outTypes.length(); + if ( spare < -1 or (starred == -1 and spare != 0) ) { + return; // Wrong number of elements to unpack. + } + for ( int i_out = 0, i_in = 0; i_out < outTypes.length(); ++i_out ) { + if ( i_out == starred ) { // PEP-3132. Made into list in assignToTuple(). + for (; spare >= 0; --spare, ++i_in ) { + auto content = indexed->typeAt(i_in).abstractType(); + outTypes[i_out] = Helper::mergeTypes(outTypes.at(i_out), content); + } + } else { + auto content = indexed->typeAt(i_in).abstractType(); + outTypes[i_out] = Helper::mergeTypes(outTypes.at(i_out), content); + ++i_in; + } + } + } else { + auto content = Helper::contentOfIterable(sourceType); + if ( !Helper::isUsefulType(content) ) { + return; + } + for (auto out = outTypes.begin(); out != outTypes.end(); ++out) { + *out = Helper::mergeTypes(*out, content); + } + } +} + +void DeclarationBuilder::assignToTuple(TupleAst* tuple, const SourceType& element) { + int starred = -1; // Index (if any) of PEP-3132 starred assignment. + for (int ii = 0; ii < tuple->elements.length(); ++ii) { + if (tuple->elements.at(ii)->astType == Ast::StarredAstType) { + starred = ii; + break; + } + } + QVector outTypes(tuple->elements.length()); + + if ( auto unsure = element.type.cast() ) { + FOREACH_FUNCTION ( const auto& type, unsure->types ) { + tryUnpackType(type.abstractType(), outTypes, starred); + } + } else { + tryUnpackType(element.type, outTypes, starred); + } + + for (int ii = 0; ii < outTypes.length(); ++ii) { + const auto sourceType = outTypes.at(ii); + auto target = tuple->elements.at(ii); if ( target->astType == Ast::StarredAstType ) { - // PEP-3132. `a, *b, c = 1, 2, 3, 4 -> b = [2, 3]` - // Starred expression is assigned a list of the values not used by unstarred ones. DUChainReadLocker lock; - auto type = ExpressionVisitor::typeObjectForIntegralType("list"); + auto listType = ExpressionVisitor::typeObjectForIntegralType("list"); lock.unlock(); - if ( !foundStarred ) { // Only allowed once, return unknown list. - // Count sources not used by other targets, slurp that many into list. - int leftovers = sources.length() - tuple->elements.length() + 1; - for ( ; leftovers > 0; leftovers--, ii++ ) { - type->addContentType(sources.at(ii).type); - } - } - source.type = AbstractType::Ptr::staticCast(type); - // List is assigned to the child expression. - target = static_cast(target)->value; - foundStarred = true; - } - else if ( ii < sources.length() ) { - source = sources.at(ii); - ii++; + listType->addContentType(sourceType); + assignToUnknown(static_cast(target)->value, listType); + } else { + assignToUnknown(target, sourceType); } - assignToUnknown(target, source); } } +void DeclarationBuilder::assignToUnknown(ExpressionAst* target, const AbstractType::Ptr type) { + auto source = SourceType{ + type, + DeclarationPointer(), + false + }; + assignToUnknown(target, source); +} + void DeclarationBuilder::assignToUnknown(ExpressionAst* target, const DeclarationBuilder::SourceType& element) { // Must be a nicer way to do this. if ( target->astType == Ast::TupleAstType ) { diff --git a/duchain/helpers.cpp b/duchain/helpers.cpp --- a/duchain/helpers.cpp +++ b/duchain/helpers.cpp @@ -509,7 +509,11 @@ return ListType::Ptr::dynamicCast(t) || IndexedContainer::Ptr::dynamicCast(t); }, [](AbstractType::Ptr t) { - if ( auto variable = ListType::Ptr::dynamicCast(t) ) { + if (auto map = MapType::Ptr::dynamicCast(t)) { + // Iterating over dicts gets keys, not values + return map->keyType().abstractType(); + } + else if ( auto variable = ListType::Ptr::dynamicCast(t) ) { return AbstractType::Ptr(variable->contentType().abstractType()); } else { diff --git a/duchain/tests/pyduchaintest.cpp b/duchain/tests/pyduchaintest.cpp --- a/duchain/tests/pyduchaintest.cpp +++ b/duchain/tests/pyduchaintest.cpp @@ -789,7 +789,6 @@ visitor->searchingForType = expectedType; visitor->visitCode(m_ast.data()); QEXPECT_FAIL("lambda", "not implemented: aliasing lambdas", Continue); - QEXPECT_FAIL("tuple_unsure", "not implemented, rework DeclarationBuilder::sourcesOfAssignment", Continue); QCOMPARE(visitor->found, true); } @@ -881,7 +880,6 @@ QTest::newRow("tuple_nested2") << "mytuple = 3, ('foo', 5.5)\nfoobar, (checkme, other) = mytuple" << "str"; QTest::newRow("tuple_nested3") << "mytuple = ((7, 'foo'), 5.5), 3\n((baz, checkme), other), foo = mytuple" << "str"; - // This isn't actually defined behaviour, but it works in CPython, so people use it... QTest::newRow("tuple_nested_ext") << "mytuple = (2, ('foo', 'bar', 6), 7)\na, (b, *checkme, c), *d = mytuple" << "list of str"; QTest::newRow("tuple_multi_assign") << "mytuple = 2, 'foo'\ncheckme = a = mytuple" << "tuple"; @@ -927,7 +925,7 @@ QTest::newRow("args_type") << "def myfun(*args): return args[0]\ncheckme = myfun(3)" << "int"; QTest::newRow("kwarg_type") << "def myfun(**args): return args[0]\ncheckme = myfun(a=3)" << "int"; - QTest::newRow("tuple_unsure") << "q = (3, str())\nq=(str(), 3)\ncheckme, _ = q" << "unsure(int, str)"; + QTest::newRow("tuple_unsure") << "q = (3, str())\nq=(str(), 3)\ncheckme, _ = q" << "unsure (int, str)"; QTest::newRow("call_class") << "class Foo:\n" " def __call__(self):\n" @@ -1382,6 +1380,10 @@ QTest::newRow("for_loop_tuple_2") << "d = [(3, 3.5)]\nfor a, b in d:\n checkme = b" << "float" << true; QTest::newRow("for_loop_tuple_unsure") << "d = [(3, 3.5), (3.5, 3)]\nfor a, b in d:\n checkme = b" << "unsure (float, int)" << true; + // Proposed by Nicolás Alvarez; why not? https://bugs.kde.org/show_bug.cgi?id=359915 + QTest::newRow("comprehension_messy") << "users = {'a':19, 'b':42, 'c':35}\n" + "sorted_list = sorted(users.items(), key=lambda kv: (-kv[1], kv[0]))\n" + "checkme = [k for r,(k,v) in enumerate(sorted_list, 1)]" << "list of str" << true; } void PyDUChainTest::testVariableCreation() @@ -1430,8 +1432,10 @@ QTest::newRow("for_loop_simple") << "for i in range(3): pass" << QStringList{"i"} << QStringList{"int"}; QTest::newRow("for_loop_unpack") << "for a, b in [(3, 5.1)]: pass" << QStringList{"a", "b"} << QStringList{"int", "float"}; - QTest::newRow("for_loop_stacked") << "for a, (b, c) in [(1, 2, 3.5)]: pass" << QStringList{"a", "b", "c"} + QTest::newRow("for_loop_stacked") << "for a, (b, c) in [(1, (2, 3.5))]: pass" << QStringList{"a", "b", "c"} << QStringList{"int", "int", "float"}; + QTest::newRow("for_loop_tuple") << "for a in 1, 2: pass" << QStringList{"a"} << QStringList{"int"}; + QTest::newRow("for_loop_dict") << "for a in {'foo': 1}: pass" << QStringList{"a"} << QStringList{"str"}; } void PyDUChainTest::testCleanupMultiplePasses()