diff --git a/codecompletion/context.cpp b/codecompletion/context.cpp --- a/codecompletion/context.cpp +++ b/codecompletion/context.cpp @@ -645,8 +645,7 @@ return items; } - Declaration* existing = Helper::declarationForName(QualifiedIdentifier(components.first()), - RangeInRevision(m_position, m_position), + Declaration* existing = Helper::declarationForName(components.first(), m_position, DUChainPointer(m_duContext.data())); if ( existing ) { // There's already a declaration for the first component; no need to suggest it diff --git a/duchain/declarationbuilder.cpp b/duchain/declarationbuilder.cpp --- a/duchain/declarationbuilder.cpp +++ b/duchain/declarationbuilder.cpp @@ -460,29 +460,9 @@ } } -void spoofNodePosition(Ast* node, const CursorInRevision& pos) { - // Ridiculous hack, see next comment. - node->startLine = node->endLine = pos.line; - node->startCol = node->endCol = pos.column - 1; - 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, pos); - } - } -} - 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 - spoofNodePosition(node->target, currentContext()->range().start); - ExpressionVisitor v(currentContext()); v.visitNode(node->iterator); assignToUnknown(node->target, Helper::contentOfIterable(v.lastType(), topContext())); diff --git a/duchain/expressionvisitor.cpp b/duchain/expressionvisitor.cpp --- a/duchain/expressionvisitor.cpp +++ b/duchain/expressionvisitor.cpp @@ -640,19 +640,19 @@ void ExpressionVisitor::visitName(Python::NameAst* node) { - RangeInRevision range; + CursorInRevision findNameBefore; if ( m_scanUntilCursor.isValid() ) { - range = RangeInRevision(CursorInRevision(0, 0), m_scanUntilCursor); + findNameBefore = m_scanUntilCursor; } else if ( m_forceGlobalSearching ) { - range = RangeInRevision::invalid(); + findNameBefore = CursorInRevision::invalid(); } else { - range = RangeInRevision(0, 0, node->endLine, node->endCol); + findNameBefore = CursorInRevision(node->endLine, node->endCol); } DUChainReadLocker lock; - Declaration* d = Helper::declarationForName(QualifiedIdentifier(node->identifier->value), - range, DUChainPointer(context())); + Declaration* d = Helper::declarationForName(node, findNameBefore, + DUChainPointer(context())); if ( d ) { bool isAlias = dynamic_cast(d) || d->isFunctionDeclaration() || dynamic_cast(d); diff --git a/duchain/helpers.h b/duchain/helpers.h --- a/duchain/helpers.h +++ b/duchain/helpers.h @@ -212,7 +212,10 @@ **/ static Declaration* resolveAliasDeclaration(Declaration* decl); - static Declaration* declarationForName(const QualifiedIdentifier& identifier, const RangeInRevision& nodeRange, + static Declaration* declarationForName(const QString& name, const CursorInRevision& location, + DUChainPointer context); + + static Declaration* declarationForName(const Python::NameAst* name, CursorInRevision location, DUChainPointer context); static QString getPythonExecutablePath(IProject* project); diff --git a/duchain/helpers.cpp b/duchain/helpers.cpp --- a/duchain/helpers.cpp +++ b/duchain/helpers.cpp @@ -176,11 +176,12 @@ return { dynamic_cast(attr), isAlias }; } -Declaration* Helper::declarationForName(const QualifiedIdentifier& identifier, const RangeInRevision& nodeRange, +Declaration* Helper::declarationForName(const QString& name, const CursorInRevision& location, KDevelop::DUChainPointer context) { DUChainReadLocker lock(DUChain::lock()); - auto localDeclarations = context->findLocalDeclarations(identifier.last(), nodeRange.end, 0, + auto identifier = KDevelop::Identifier(name); + auto localDeclarations = context->findLocalDeclarations(identifier, location, 0, AbstractType::Ptr(0), DUContext::DontResolveAliases); if ( !localDeclarations.isEmpty() ) { return localDeclarations.last(); @@ -191,8 +192,8 @@ bool findInNext = true, findBeyondUse = false; do { if (findInNext) { - CursorInRevision findUntil = findBeyondUse ? currentContext->topContext()->range().end : nodeRange.end; - declarations = currentContext->findDeclarations(identifier.last(), findUntil); + CursorInRevision findUntil = findBeyondUse ? currentContext->topContext()->range().end : location; + declarations = currentContext->findDeclarations(identifier, findUntil); for (Declaration* declaration: declarations) { if (declaration->context()->type() != DUContext::Class || @@ -220,6 +221,29 @@ return nullptr; } + +Declaration* Helper::declarationForName(const Python::NameAst* name, CursorInRevision location, + KDevelop::DUChainPointer context) +{ + const Ast* checkNode = name; + while ((checkNode = checkNode->parent)) { + switch (checkNode->astType) { + default: + continue; + case Ast::ListComprehensionAstType: + case Ast::SetComprehensionAstType: + case Ast::DictionaryComprehensionAstType: + case Ast::GeneratorExpressionAstType: + // Variables in comprehensions are used before their definition. `[foo for foo in bar]` + auto cmpEnd = CursorInRevision(checkNode->endLine, checkNode->endCol); + if (cmpEnd > location) { + location = cmpEnd; + } + } + } + return declarationForName(name->identifier->value, location, context); +} + QVector Helper::internalContextsForClass(const StructureType::Ptr classType, const TopDUContext* context, ContextSearchFlags flags, int depth) { diff --git a/duchain/tests/pyduchaintest.cpp b/duchain/tests/pyduchaintest.cpp --- a/duchain/tests/pyduchaintest.cpp +++ b/duchain/tests/pyduchaintest.cpp @@ -1341,6 +1341,7 @@ QTest::newRow("misplaced_return_class") << "class A:\n return 25" << 1; QTest::newRow("correct_return") << "def foo():\n return" << 0; QTest::newRow("lambda_argument_outside") << "def bar():\n lambda foo: 3\n foo" << 1; + QTest::newRow("use_found_at_decl") << "foo = 3" << 0; } void PyDUChainTest::testImportDeclarations_data() { @@ -1602,6 +1603,8 @@ QVERIFY(decls.first()->abstractType()); QEXPECT_FAIL("dict_of_int_call", "returnContentEqualsContentOf isn't suitable", Continue); QEXPECT_FAIL("dict_from_tuples", "returnContentEqualsContentOf isn't suitable", Continue); + QEXPECT_FAIL("comprehension_shadowing_ms", "Nothing is foolproof to a sufficiently capable fool", Continue); + QEXPECT_FAIL("comprehension_shadowing_nest2", "See above", Continue); QCOMPARE(decls.first()->abstractType()->toString(), contenttype); } } @@ -1665,6 +1668,14 @@ "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; QTest::newRow("comprehension_multiline") << "checkme = [a for\n a in \n (1, 2)]" << "list of int" << true; + QTest::newRow("comprehension_multistage") << "nested = (1, 2), (3, 4)\n" + "checkme = [foo for bar in nested for foo in bar]" << "list of int" << true; + QTest::newRow("comprehension_shadowing_ms") << "nested = (1, 2), (3, 4)\n" + "checkme = [foo for foo in nested for foo in foo]" << "list of int" << true; + QTest::newRow("comprehension_shadowing_nest1") << "nested = (1, 2), (3, 4)\n" + "checkme = [foo for foo in [foo for foo in nested]]" << "list of tuple of (int, int)" << true; + QTest::newRow("comprehension_shadowing_nest2") << "nested = (1, 2), (3, 4)\n" + "checkme = [[foo for foo in foo] for foo in nested]" << "list of list of int" << true; // From https://bugs.kde.org/show_bug.cgi?id=359912 QTest::newRow("subscript_multi") << "class Middle:\n def __getitem__(self, key):\n return str()\n" diff --git a/duchain/usebuilder.cpp b/duchain/usebuilder.cpp --- a/duchain/usebuilder.cpp +++ b/duchain/usebuilder.cpp @@ -73,8 +73,7 @@ void UseBuilder::visitName(NameAst* node) { DUContext* context = contextAtOrCurrent(editorFindPositionSafe(node)); - Declaration* declaration = Helper::declarationForName(identifierForNode(node->identifier), - editorFindRange(node, node), + Declaration* declaration = Helper::declarationForName(node, editorFindPositionSafe(node), DUChainPointer(context)); Q_ASSERT(node->identifier);