From 977702d933b4fd19051f9e3893f8fe64b3d2a427 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Thu, 16 May 2024 17:08:35 +0200 Subject: [PATCH] CppEditor: Move CompleteSwitchStatement quickfix into its own files Change-Id: Ibb4032c4c9715af62f644e84985712b68a11fdd9 Reviewed-by: Reviewed-by: Christian Stenger --- src/plugins/cppeditor/CMakeLists.txt | 1 + src/plugins/cppeditor/cppeditor.qbs | 2 + .../quickfixes/completeswitchstatement.cpp | 799 ++++++++++++++++++ .../quickfixes/completeswitchstatement.h | 8 + .../cppeditor/quickfixes/cppquickfix_test.cpp | 580 +------------ .../cppeditor/quickfixes/cppquickfixes.cpp | 182 +--- .../cppeditor/quickfixes/cppquickfixes.h | 12 - 7 files changed, 813 insertions(+), 771 deletions(-) create mode 100644 src/plugins/cppeditor/quickfixes/completeswitchstatement.cpp create mode 100644 src/plugins/cppeditor/quickfixes/completeswitchstatement.h diff --git a/src/plugins/cppeditor/CMakeLists.txt b/src/plugins/cppeditor/CMakeLists.txt index 65c8820f65f..1011af9eedf 100644 --- a/src/plugins/cppeditor/CMakeLists.txt +++ b/src/plugins/cppeditor/CMakeLists.txt @@ -97,6 +97,7 @@ add_qtc_plugin(CppEditor projectpart.cpp projectpart.h quickfixes/assigntolocalvariable.cpp quickfixes/assigntolocalvariable.h quickfixes/bringidentifierintoscope.cpp quickfixes/bringidentifierintoscope.h + quickfixes/completeswitchstatement.cpp quickfixes/completeswitchstatement.h quickfixes/convertqt4connect.cpp quickfixes/convertqt4connect.h quickfixes/convertstringliteral.cpp quickfixes/convertstringliteral.h quickfixes/cppcodegenerationquickfixes.cpp quickfixes/cppcodegenerationquickfixes.h diff --git a/src/plugins/cppeditor/cppeditor.qbs b/src/plugins/cppeditor/cppeditor.qbs index 095c6d4f424..6065b517f82 100644 --- a/src/plugins/cppeditor/cppeditor.qbs +++ b/src/plugins/cppeditor/cppeditor.qbs @@ -223,6 +223,8 @@ QtcPlugin { "assigntolocalvariable.h", "bringidentifierintoscope.cpp", "bringidentifierintoscope.h", + "completeswitchstatement.cpp", + "completeswitchstatement.h", "convertfromandtopointer.cpp", "convertfromandtopointer.h", "convertqt4connect.cpp", diff --git a/src/plugins/cppeditor/quickfixes/completeswitchstatement.cpp b/src/plugins/cppeditor/quickfixes/completeswitchstatement.cpp new file mode 100644 index 00000000000..f92f5bbfd4c --- /dev/null +++ b/src/plugins/cppeditor/quickfixes/completeswitchstatement.cpp @@ -0,0 +1,799 @@ +// Copyright (C) 2024 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +#include "completeswitchstatement.h" + +#include "../cppeditortr.h" +#include "../cpprefactoringchanges.h" +#include "cppquickfix.h" + +#include +#include + +#ifdef WITH_TESTS +#include "cppquickfix_test.h" +#include +#endif + +using namespace CPlusPlus; +using namespace Utils; + +namespace CppEditor::Internal { +namespace { + +class CaseStatementCollector : public ASTVisitor +{ +public: + CaseStatementCollector(Document::Ptr document, const Snapshot &snapshot, + Scope *scope) + : ASTVisitor(document->translationUnit()), + document(document), + scope(scope) + { + typeOfExpression.init(document, snapshot); + } + + QStringList operator ()(AST *ast) + { + values.clear(); + foundCaseStatementLevel = false; + accept(ast); + return values; + } + + bool preVisit(AST *ast) override { + if (CaseStatementAST *cs = ast->asCaseStatement()) { + foundCaseStatementLevel = true; + if (ExpressionAST *csExpression = cs->expression) { + if (ExpressionAST *expression = csExpression->asIdExpression()) { + QList candidates = typeOfExpression(expression, document, scope); + if (!candidates.isEmpty() && candidates.first().declaration()) { + Symbol *decl = candidates.first().declaration(); + values << prettyPrint.prettyName(LookupContext::fullyQualifiedName(decl)); + } + } + } + return true; + } else if (foundCaseStatementLevel) { + return false; + } + return true; + } + + Overview prettyPrint; + bool foundCaseStatementLevel = false; + QStringList values; + TypeOfExpression typeOfExpression; + Document::Ptr document; + Scope *scope; +}; + +class CompleteSwitchCaseStatementOp: public CppQuickFixOperation +{ +public: + CompleteSwitchCaseStatementOp(const CppQuickFixInterface &interface, + int priority, CompoundStatementAST *compoundStatement, const QStringList &values) + : CppQuickFixOperation(interface, priority) + , compoundStatement(compoundStatement) + , values(values) + { + setDescription(Tr::tr("Complete Switch Statement")); + } + + void perform() override + { + CppRefactoringChanges refactoring(snapshot()); + CppRefactoringFilePtr currentFile = refactoring.cppFile(filePath()); + + ChangeSet changes; + int start = currentFile->endOf(compoundStatement->lbrace_token); + changes.insert(start, QLatin1String("\ncase ") + + values.join(QLatin1String(":\nbreak;\ncase ")) + + QLatin1String(":\nbreak;")); + currentFile->setChangeSet(changes); + currentFile->apply(); + } + + CompoundStatementAST *compoundStatement; + QStringList values; +}; + +static Enum *findEnum(const QList &results, const LookupContext &ctxt) +{ + for (const LookupItem &result : results) { + const FullySpecifiedType fst = result.type(); + + Type *type = result.declaration() ? result.declaration()->type().type() + : fst.type(); + + if (!type) + continue; + if (Enum *e = type->asEnumType()) + return e; + if (const NamedType *namedType = type->asNamedType()) { + if (ClassOrNamespace *con = ctxt.lookupType(namedType->name(), result.scope())) { + QList enums = con->unscopedEnums(); + const QList symbols = con->symbols(); + for (Symbol * const s : symbols) { + if (const auto e = s->asEnum()) + enums << e; + } + const Name *referenceName = namedType->name(); + if (const QualifiedNameId *qualifiedName = referenceName->asQualifiedNameId()) + referenceName = qualifiedName->name(); + for (Enum *e : std::as_const(enums)) { + if (const Name *candidateName = e->name()) { + if (candidateName->match(referenceName)) + return e; + } + } + } + } + } + + return nullptr; +} + +Enum *conditionEnum(const CppQuickFixInterface &interface, SwitchStatementAST *statement) +{ + Block *block = statement->symbol; + Scope *scope = interface.semanticInfo().doc->scopeAt(block->line(), block->column()); + TypeOfExpression typeOfExpression; + typeOfExpression.setExpandTemplates(true); + typeOfExpression.init(interface.semanticInfo().doc, interface.snapshot()); + const QList results = typeOfExpression(statement->condition, + interface.semanticInfo().doc, + scope); + + return findEnum(results, typeOfExpression.context()); +} + +//! Adds missing case statements for "switch (enumVariable)" +class CompleteSwitchStatement: public CppQuickFixFactory +{ +public: + CompleteSwitchStatement() { setClangdReplacement({12}); } +#ifdef WITH_TESTS + static QObject *createTest(); +#endif + +private: + void doMatch(const CppQuickFixInterface &interface, QuickFixOperations &result) override + { + const QList &path = interface.path(); + + if (path.isEmpty()) + return; + + // look for switch statement + for (int depth = path.size() - 1; depth >= 0; --depth) { + AST *ast = path.at(depth); + SwitchStatementAST *switchStatement = ast->asSwitchStatement(); + if (switchStatement) { + if (!switchStatement->statement || !switchStatement->symbol) + return; + CompoundStatementAST *compoundStatement = switchStatement->statement->asCompoundStatement(); + if (!compoundStatement) // we ignore pathologic case "switch (t) case A: ;" + return; + // look if the condition's type is an enum + if (Enum *e = conditionEnum(interface, switchStatement)) { + // check the possible enum values + QStringList values; + Overview prettyPrint; + for (int i = 0; i < e->memberCount(); ++i) { + if (Declaration *decl = e->memberAt(i)->asDeclaration()) + values << prettyPrint.prettyName(LookupContext::fullyQualifiedName(decl)); + } + // Get the used values + Block *block = switchStatement->symbol; + CaseStatementCollector caseValues(interface.semanticInfo().doc, interface.snapshot(), + interface.semanticInfo().doc->scopeAt(block->line(), block->column())); + const QStringList usedValues = caseValues(switchStatement); + // save the values that would be added + for (const QString &usedValue : usedValues) + values.removeAll(usedValue); + if (!values.isEmpty()) + result << new CompleteSwitchCaseStatementOp(interface, depth, + compoundStatement, values); + return; + } + + return; + } + } + } +}; + +#ifdef WITH_TESTS +using namespace Tests; + +class CompleteSwitchStatementTest : public QObject +{ + Q_OBJECT + +private slots: + void test_data() + { + QTest::addColumn("original"); + QTest::addColumn("expected"); + + using QByteArray = QByteArray; + + // Checks: All enum values are added as case statements for a blank switch. + QTest::newRow("basic1") + << QByteArray( + "enum EnumType { V1, V2 };\n" + "\n" + "void f()\n" + "{\n" + " EnumType t;\n" + " @switch (t) {\n" + " }\n" + "}\n") + << QByteArray( + "enum EnumType { V1, V2 };\n" + "\n" + "void f()\n" + "{\n" + " EnumType t;\n" + " switch (t) {\n" + " case V1:\n" + " break;\n" + " case V2:\n" + " break;\n" + " }\n" + "}\n"); + + // Same as above for enum class. + QTest::newRow("basic1_enum class") + << QByteArray( + "enum class EnumType { V1, V2 };\n" + "\n" + "void f()\n" + "{\n" + " EnumType t;\n" + " @switch (t) {\n" + " }\n" + "}\n") + << QByteArray( + "enum class EnumType { V1, V2 };\n" + "\n" + "void f()\n" + "{\n" + " EnumType t;\n" + " switch (t) {\n" + " case EnumType::V1:\n" + " break;\n" + " case EnumType::V2:\n" + " break;\n" + " }\n" + "}\n"); + + // Same as above with the cursor somewhere in the body. + QTest::newRow("basic1_enum class, cursor in the body") + << QByteArray( + "enum class EnumType { V1, V2 };\n" + "\n" + "void f()\n" + "{\n" + " EnumType t;\n" + " switch (t) {\n" + " @}\n" + "}\n") + << QByteArray( + "enum class EnumType { V1, V2 };\n" + "\n" + "void f()\n" + "{\n" + " EnumType t;\n" + " switch (t) {\n" + " case EnumType::V1:\n" + " break;\n" + " case EnumType::V2:\n" + " break;\n" + " }\n" + "}\n"); + + // Checks: All enum values are added as case statements for a blank switch when + // the variable is declared alongside the enum definition. + QTest::newRow("basic1_enum_with_declaration") + << QByteArray( + "enum EnumType { V1, V2 } t;\n" + "\n" + "void f()\n" + "{\n" + " @switch (t) {\n" + " }\n" + "}\n") + << QByteArray( + "enum EnumType { V1, V2 } t;\n" + "\n" + "void f()\n" + "{\n" + " switch (t) {\n" + " case V1:\n" + " break;\n" + " case V2:\n" + " break;\n" + " }\n" + "}\n"); + + // Same as above for enum class. + QTest::newRow("basic1_enum_with_declaration_enumClass") + << QByteArray( + "enum class EnumType { V1, V2 } t;\n" + "\n" + "void f()\n" + "{\n" + " @switch (t) {\n" + " }\n" + "}\n") + << QByteArray( + "enum class EnumType { V1, V2 } t;\n" + "\n" + "void f()\n" + "{\n" + " switch (t) {\n" + " case EnumType::V1:\n" + " break;\n" + " case EnumType::V2:\n" + " break;\n" + " }\n" + "}\n"); + + // Checks: All enum values are added as case statements for a blank switch + // for anonymous enums. + QTest::newRow("basic1_anonymous_enum") + << QByteArray( + "enum { V1, V2 } t;\n" + "\n" + "void f()\n" + "{\n" + " @switch (t) {\n" + " }\n" + "}\n") + << QByteArray( + "enum { V1, V2 } t;\n" + "\n" + "void f()\n" + "{\n" + " switch (t) {\n" + " case V1:\n" + " break;\n" + " case V2:\n" + " break;\n" + " }\n" + "}\n"); + + // Checks: All enum values are added as case statements for a blank switch with a default case. + QTest::newRow("basic2") + << QByteArray( + "enum EnumType { V1, V2 };\n" + "\n" + "void f()\n" + "{\n" + " EnumType t;\n" + " @switch (t) {\n" + " default:\n" + " break;\n" + " }\n" + "}\n") + << QByteArray( + "enum EnumType { V1, V2 };\n" + "\n" + "void f()\n" + "{\n" + " EnumType t;\n" + " switch (t) {\n" + " case V1:\n" + " break;\n" + " case V2:\n" + " break;\n" + " default:\n" + " break;\n" + " }\n" + "}\n"); + + // Same as above for enum class. + QTest::newRow("basic2_enumClass") + << QByteArray( + "enum class EnumType { V1, V2 };\n" + "\n" + "void f()\n" + "{\n" + " EnumType t;\n" + " @switch (t) {\n" + " default:\n" + " break;\n" + " }\n" + "}\n") + << QByteArray( + "enum class EnumType { V1, V2 };\n" + "\n" + "void f()\n" + "{\n" + " EnumType t;\n" + " switch (t) {\n" + " case EnumType::V1:\n" + " break;\n" + " case EnumType::V2:\n" + " break;\n" + " default:\n" + " break;\n" + " }\n" + "}\n"); + + // Checks: Enum type in class is found. + QTest::newRow("enumTypeInClass") + << QByteArray( + "struct C { enum EnumType { V1, V2 }; };\n" + "\n" + "void f(C::EnumType t) {\n" + " @switch (t) {\n" + " }\n" + "}\n") + << QByteArray( + "struct C { enum EnumType { V1, V2 }; };\n" + "\n" + "void f(C::EnumType t) {\n" + " switch (t) {\n" + " case C::V1:\n" + " break;\n" + " case C::V2:\n" + " break;\n" + " }\n" + "}\n"); + + // Same as above for enum class. + QTest::newRow("enumClassInClass") + << QByteArray( + "struct C { enum class EnumType { V1, V2 }; };\n" + "\n" + "void f(C::EnumType t) {\n" + " @switch (t) {\n" + " }\n" + "}\n") + << QByteArray( + "struct C { enum class EnumType { V1, V2 }; };\n" + "\n" + "void f(C::EnumType t) {\n" + " switch (t) {\n" + " case C::EnumType::V1:\n" + " break;\n" + " case C::EnumType::V2:\n" + " break;\n" + " }\n" + "}\n"); + + // Checks: Enum type in namespace is found. + QTest::newRow("enumTypeInNamespace") + << QByteArray( + "namespace N { enum EnumType { V1, V2 }; };\n" + "\n" + "void f(N::EnumType t) {\n" + " @switch (t) {\n" + " }\n" + "}\n") + << QByteArray( + "namespace N { enum EnumType { V1, V2 }; };\n" + "\n" + "void f(N::EnumType t) {\n" + " switch (t) {\n" + " case N::V1:\n" + " break;\n" + " case N::V2:\n" + " break;\n" + " }\n" + "}\n"); + + // Same as above for enum class. + QTest::newRow("enumClassInNamespace") + << QByteArray( + "namespace N { enum class EnumType { V1, V2 }; };\n" + "\n" + "void f(N::EnumType t) {\n" + " @switch (t) {\n" + " }\n" + "}\n") + << QByteArray( + "namespace N { enum class EnumType { V1, V2 }; };\n" + "\n" + "void f(N::EnumType t) {\n" + " switch (t) {\n" + " case N::EnumType::V1:\n" + " break;\n" + " case N::EnumType::V2:\n" + " break;\n" + " }\n" + "}\n"); + + // Checks: The missing enum value is added. + QTest::newRow("oneValueMissing") + << QByteArray( + "enum EnumType { V1, V2 };\n" + "\n" + "void f()\n" + "{\n" + " EnumType t;\n" + " @switch (t) {\n" + " case V2:\n" + " break;\n" + " default:\n" + " break;\n" + " }\n" + "}\n") + << QByteArray( + "enum EnumType { V1, V2 };\n" + "\n" + "void f()\n" + "{\n" + " EnumType t;\n" + " switch (t) {\n" + " case V1:\n" + " break;\n" + " case V2:\n" + " break;\n" + " default:\n" + " break;\n" + " }\n" + "}\n"); + + // Checks: Same as above for enum class. + QTest::newRow("oneValueMissing_enumClass") + << QByteArray( + "enum class EnumType { V1, V2 };\n" + "\n" + "void f()\n" + "{\n" + " EnumType t;\n" + " @switch (t) {\n" + " case EnumType::V2:\n" + " break;\n" + " default:\n" + " break;\n" + " }\n" + "}\n") + << QByteArray( + "enum class EnumType { V1, V2 };\n" + "\n" + "void f()\n" + "{\n" + " EnumType t;\n" + " switch (t) {\n" + " case EnumType::V1:\n" + " break;\n" + " case EnumType::V2:\n" + " break;\n" + " default:\n" + " break;\n" + " }\n" + "}\n"); + + // Checks: Find the correct enum type despite there being a declaration with the same name. + QTest::newRow("QTCREATORBUG10366_1") + << QByteArray( + "enum test { TEST_1, TEST_2 };\n" + "\n" + "void f() {\n" + " enum test test;\n" + " @switch (test) {\n" + " }\n" + "}\n") + << QByteArray( + "enum test { TEST_1, TEST_2 };\n" + "\n" + "void f() {\n" + " enum test test;\n" + " switch (test) {\n" + " case TEST_1:\n" + " break;\n" + " case TEST_2:\n" + " break;\n" + " }\n" + "}\n"); + + // Same as above for enum class. + QTest::newRow("QTCREATORBUG10366_1_enumClass") + << QByteArray( + "enum class test { TEST_1, TEST_2 };\n" + "\n" + "void f() {\n" + " enum test test;\n" + " @switch (test) {\n" + " }\n" + "}\n") + << QByteArray( + "enum class test { TEST_1, TEST_2 };\n" + "\n" + "void f() {\n" + " enum test test;\n" + " switch (test) {\n" + " case test::TEST_1:\n" + " break;\n" + " case test::TEST_2:\n" + " break;\n" + " }\n" + "}\n"); + + // Checks: Find the correct enum type despite there being a declaration with the same name. + QTest::newRow("QTCREATORBUG10366_2") + << QByteArray( + "enum test1 { Wrong11, Wrong12 };\n" + "enum test { Right1, Right2 };\n" + "enum test2 { Wrong21, Wrong22 };\n" + "\n" + "int main() {\n" + " enum test test;\n" + " @switch (test) {\n" + " }\n" + "}\n") + << QByteArray( + "enum test1 { Wrong11, Wrong12 };\n" + "enum test { Right1, Right2 };\n" + "enum test2 { Wrong21, Wrong22 };\n" + "\n" + "int main() {\n" + " enum test test;\n" + " switch (test) {\n" + " case Right1:\n" + " break;\n" + " case Right2:\n" + " break;\n" + " }\n" + "}\n"); + + // Same as above for enum class. + QTest::newRow("QTCREATORBUG10366_2_enumClass") + << QByteArray( + "enum class test1 { Wrong11, Wrong12 };\n" + "enum class test { Right1, Right2 };\n" + "enum class test2 { Wrong21, Wrong22 };\n" + "\n" + "int main() {\n" + " enum test test;\n" + " @switch (test) {\n" + " }\n" + "}\n") + << QByteArray( + "enum class test1 { Wrong11, Wrong12 };\n" + "enum class test { Right1, Right2 };\n" + "enum class test2 { Wrong21, Wrong22 };\n" + "\n" + "int main() {\n" + " enum test test;\n" + " switch (test) {\n" + " case test::Right1:\n" + " break;\n" + " case test::Right2:\n" + " break;\n" + " }\n" + "}\n"); + + // Checks: Do not crash on incomplete case statetement. + QTest::newRow("doNotCrashOnIncompleteCase") + << QByteArray( + "enum E {};\n" + "void f(E o)\n" + "{\n" + " @switch (o)\n" + " {\n" + " case\n" + " }\n" + "}\n") + << QByteArray(); + + // Same as above for enum class. + QTest::newRow("doNotCrashOnIncompleteCase_enumClass") + << QByteArray( + "enum class E {};\n" + "void f(E o)\n" + "{\n" + " @switch (o)\n" + " {\n" + " case\n" + " }\n" + "}\n") + << QByteArray(); + + // Checks: complete switch statement where enum is goes via a template type parameter + QTest::newRow("QTCREATORBUG-24752") + << QByteArray( + "enum E {EA, EB};\n" + "template struct S {\n" + " static T theType() { return T(); }\n" + "};\n" + "int main() {\n" + " @switch (S::theType()) {\n" + " }\n" + "}\n") + << QByteArray( + "enum E {EA, EB};\n" + "template struct S {\n" + " static T theType() { return T(); }\n" + "};\n" + "int main() {\n" + " switch (S::theType()) {\n" + " case EA:\n" + " break;\n" + " case EB:\n" + " break;\n" + " }\n" + "}\n"); + + // Same as above for enum class. + QTest::newRow("QTCREATORBUG-24752_enumClass") + << QByteArray( + "enum class E {A, B};\n" + "template struct S {\n" + " static T theType() { return T(); }\n" + "};\n" + "int main() {\n" + " @switch (S::theType()) {\n" + " }\n" + "}\n") + << QByteArray( + "enum class E {A, B};\n" + "template struct S {\n" + " static T theType() { return T(); }\n" + "};\n" + "int main() {\n" + " switch (S::theType()) {\n" + " case E::A:\n" + " break;\n" + " case E::B:\n" + " break;\n" + " }\n" + "}\n"); + + // Checks: Complete switch statement where enum is return type of a template function + // which is outside the scope of the return value. + // TODO: Type minimization. + QTest::newRow("QTCREATORBUG-25998") + << QByteArray( + "template T enumCast(int value) { return static_cast(value); }\n" + "class Test {\n" + " enum class E { V1, V2 };" + " void func(int i) {\n" + " @switch (enumCast(i)) {\n" + " }\n" + " }\n" + "};\n") + << QByteArray( + "template T enumCast(int value) { return static_cast(value); }\n" + "class Test {\n" + " enum class E { V1, V2 };" + " void func(int i) {\n" + " switch (enumCast(i)) {\n" + " case Test::E::V1:\n" + " break;\n" + " case Test::E::V2:\n" + " break;\n" + " }\n" + " }\n" + "};\n"); + } + + void test() + { + QFETCH(QByteArray, original); + QFETCH(QByteArray, expected); + CompleteSwitchStatement factory; + QuickFixOperationTest(singleDocument(original, expected), &factory); + } +}; + +QObject *CompleteSwitchStatement::createTest() { return new CompleteSwitchStatementTest; } + +#endif // WITH_TESTS +} // namespace + +void registerCompleteSwitchStatementQuickfix() +{ + CppQuickFixFactory::registerFactory(); +} + +} // namespace CppEditor::Internal + +#ifdef WITH_TESTS +#include +#endif diff --git a/src/plugins/cppeditor/quickfixes/completeswitchstatement.h b/src/plugins/cppeditor/quickfixes/completeswitchstatement.h new file mode 100644 index 00000000000..dc2293b1b24 --- /dev/null +++ b/src/plugins/cppeditor/quickfixes/completeswitchstatement.h @@ -0,0 +1,8 @@ +// Copyright (C) 2024 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +#pragma once + +namespace CppEditor::Internal { +void registerCompleteSwitchStatementQuickfix(); +} // namespace CppEditor::Internal diff --git a/src/plugins/cppeditor/quickfixes/cppquickfix_test.cpp b/src/plugins/cppeditor/quickfixes/cppquickfix_test.cpp index 08351d6d42f..f949ed1b025 100644 --- a/src/plugins/cppeditor/quickfixes/cppquickfix_test.cpp +++ b/src/plugins/cppeditor/quickfixes/cppquickfix_test.cpp @@ -206,7 +206,7 @@ QuickFixOperationTest::QuickFixOperationTest(const QList &testD QuickFixOperations operations; factory->match(quickFixInterface, operations); if (operations.isEmpty()) { - QEXPECT_FAIL("CompleteSwitchCaseStatement_QTCREATORBUG-25998", "FIXME", Abort); + QEXPECT_FAIL("QTCREATORBUG-25998", "FIXME", Abort); QVERIFY(testDocuments.first()->m_expectedSource.isEmpty()); return; } @@ -263,584 +263,6 @@ void QuickfixTest::testGeneric_data() QTest::addColumn("original"); QTest::addColumn("expected"); - // Checks: All enum values are added as case statements for a blank switch. - QTest::newRow("CompleteSwitchCaseStatement_basic1") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "enum EnumType { V1, V2 };\n" - "\n" - "void f()\n" - "{\n" - " EnumType t;\n" - " @switch (t) {\n" - " }\n" - "}\n" - ) << _( - "enum EnumType { V1, V2 };\n" - "\n" - "void f()\n" - "{\n" - " EnumType t;\n" - " switch (t) {\n" - " case V1:\n" - " break;\n" - " case V2:\n" - " break;\n" - " }\n" - "}\n" - ); - - // Same as above for enum class. - QTest::newRow("CompleteSwitchCaseStatement_basic1_enum class") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "enum class EnumType { V1, V2 };\n" - "\n" - "void f()\n" - "{\n" - " EnumType t;\n" - " @switch (t) {\n" - " }\n" - "}\n" - ) << _( - "enum class EnumType { V1, V2 };\n" - "\n" - "void f()\n" - "{\n" - " EnumType t;\n" - " switch (t) {\n" - " case EnumType::V1:\n" - " break;\n" - " case EnumType::V2:\n" - " break;\n" - " }\n" - "}\n" - ); - - // Same as above with the cursor somewhere in the body. - QTest::newRow("CompleteSwitchCaseStatement_basic1_enum class, cursor in the body") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "enum class EnumType { V1, V2 };\n" - "\n" - "void f()\n" - "{\n" - " EnumType t;\n" - " switch (t) {\n" - " @}\n" - "}\n" - ) << _( - "enum class EnumType { V1, V2 };\n" - "\n" - "void f()\n" - "{\n" - " EnumType t;\n" - " switch (t) {\n" - " case EnumType::V1:\n" - " break;\n" - " case EnumType::V2:\n" - " break;\n" - " }\n" - "}\n" - ); - - // Checks: All enum values are added as case statements for a blank switch when - // the variable is declared alongside the enum definition. - QTest::newRow("CompleteSwitchCaseStatement_basic1_enum_with_declaration") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "enum EnumType { V1, V2 } t;\n" - "\n" - "void f()\n" - "{\n" - " @switch (t) {\n" - " }\n" - "}\n" - ) << _( - "enum EnumType { V1, V2 } t;\n" - "\n" - "void f()\n" - "{\n" - " switch (t) {\n" - " case V1:\n" - " break;\n" - " case V2:\n" - " break;\n" - " }\n" - "}\n" - ); - - // Same as above for enum class. - QTest::newRow("CompleteSwitchCaseStatement_basic1_enum_with_declaration_enumClass") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "enum class EnumType { V1, V2 } t;\n" - "\n" - "void f()\n" - "{\n" - " @switch (t) {\n" - " }\n" - "}\n" - ) << _( - "enum class EnumType { V1, V2 } t;\n" - "\n" - "void f()\n" - "{\n" - " switch (t) {\n" - " case EnumType::V1:\n" - " break;\n" - " case EnumType::V2:\n" - " break;\n" - " }\n" - "}\n" - ); - - // Checks: All enum values are added as case statements for a blank switch - // for anonymous enums. - QTest::newRow("CompleteSwitchCaseStatement_basic1_anonymous_enum") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "enum { V1, V2 } t;\n" - "\n" - "void f()\n" - "{\n" - " @switch (t) {\n" - " }\n" - "}\n" - ) << _( - "enum { V1, V2 } t;\n" - "\n" - "void f()\n" - "{\n" - " switch (t) {\n" - " case V1:\n" - " break;\n" - " case V2:\n" - " break;\n" - " }\n" - "}\n" - ); - - // Checks: All enum values are added as case statements for a blank switch with a default case. - QTest::newRow("CompleteSwitchCaseStatement_basic2") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "enum EnumType { V1, V2 };\n" - "\n" - "void f()\n" - "{\n" - " EnumType t;\n" - " @switch (t) {\n" - " default:\n" - " break;\n" - " }\n" - "}\n" - ) << _( - "enum EnumType { V1, V2 };\n" - "\n" - "void f()\n" - "{\n" - " EnumType t;\n" - " switch (t) {\n" - " case V1:\n" - " break;\n" - " case V2:\n" - " break;\n" - " default:\n" - " break;\n" - " }\n" - "}\n" - ); - - // Same as above for enum class. - QTest::newRow("CompleteSwitchCaseStatement_basic2_enumClass") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "enum class EnumType { V1, V2 };\n" - "\n" - "void f()\n" - "{\n" - " EnumType t;\n" - " @switch (t) {\n" - " default:\n" - " break;\n" - " }\n" - "}\n" - ) << _( - "enum class EnumType { V1, V2 };\n" - "\n" - "void f()\n" - "{\n" - " EnumType t;\n" - " switch (t) {\n" - " case EnumType::V1:\n" - " break;\n" - " case EnumType::V2:\n" - " break;\n" - " default:\n" - " break;\n" - " }\n" - "}\n" - ); - - // Checks: Enum type in class is found. - QTest::newRow("CompleteSwitchCaseStatement_enumTypeInClass") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "struct C { enum EnumType { V1, V2 }; };\n" - "\n" - "void f(C::EnumType t) {\n" - " @switch (t) {\n" - " }\n" - "}\n" - ) << _( - "struct C { enum EnumType { V1, V2 }; };\n" - "\n" - "void f(C::EnumType t) {\n" - " switch (t) {\n" - " case C::V1:\n" - " break;\n" - " case C::V2:\n" - " break;\n" - " }\n" - "}\n" - ); - - // Same as above for enum class. - QTest::newRow("CompleteSwitchCaseStatement_enumClassInClass") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "struct C { enum class EnumType { V1, V2 }; };\n" - "\n" - "void f(C::EnumType t) {\n" - " @switch (t) {\n" - " }\n" - "}\n" - ) << _( - "struct C { enum class EnumType { V1, V2 }; };\n" - "\n" - "void f(C::EnumType t) {\n" - " switch (t) {\n" - " case C::EnumType::V1:\n" - " break;\n" - " case C::EnumType::V2:\n" - " break;\n" - " }\n" - "}\n" - ); - - // Checks: Enum type in namespace is found. - QTest::newRow("CompleteSwitchCaseStatement_enumTypeInNamespace") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "namespace N { enum EnumType { V1, V2 }; };\n" - "\n" - "void f(N::EnumType t) {\n" - " @switch (t) {\n" - " }\n" - "}\n" - ) << _( - "namespace N { enum EnumType { V1, V2 }; };\n" - "\n" - "void f(N::EnumType t) {\n" - " switch (t) {\n" - " case N::V1:\n" - " break;\n" - " case N::V2:\n" - " break;\n" - " }\n" - "}\n" - ); - - // Same as above for enum class. - QTest::newRow("CompleteSwitchCaseStatement_enumClassInNamespace") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "namespace N { enum class EnumType { V1, V2 }; };\n" - "\n" - "void f(N::EnumType t) {\n" - " @switch (t) {\n" - " }\n" - "}\n" - ) << _( - "namespace N { enum class EnumType { V1, V2 }; };\n" - "\n" - "void f(N::EnumType t) {\n" - " switch (t) {\n" - " case N::EnumType::V1:\n" - " break;\n" - " case N::EnumType::V2:\n" - " break;\n" - " }\n" - "}\n" - ); - - // Checks: The missing enum value is added. - QTest::newRow("CompleteSwitchCaseStatement_oneValueMissing") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "enum EnumType { V1, V2 };\n" - "\n" - "void f()\n" - "{\n" - " EnumType t;\n" - " @switch (t) {\n" - " case V2:\n" - " break;\n" - " default:\n" - " break;\n" - " }\n" - "}\n" - ) << _( - "enum EnumType { V1, V2 };\n" - "\n" - "void f()\n" - "{\n" - " EnumType t;\n" - " switch (t) {\n" - " case V1:\n" - " break;\n" - " case V2:\n" - " break;\n" - " default:\n" - " break;\n" - " }\n" - "}\n" - ); - - // Checks: Same as above for enum class. - QTest::newRow("CompleteSwitchCaseStatement_oneValueMissing_enumClass") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "enum class EnumType { V1, V2 };\n" - "\n" - "void f()\n" - "{\n" - " EnumType t;\n" - " @switch (t) {\n" - " case EnumType::V2:\n" - " break;\n" - " default:\n" - " break;\n" - " }\n" - "}\n" - ) << _( - "enum class EnumType { V1, V2 };\n" - "\n" - "void f()\n" - "{\n" - " EnumType t;\n" - " switch (t) {\n" - " case EnumType::V1:\n" - " break;\n" - " case EnumType::V2:\n" - " break;\n" - " default:\n" - " break;\n" - " }\n" - "}\n" - ); - - // Checks: Find the correct enum type despite there being a declaration with the same name. - QTest::newRow("CompleteSwitchCaseStatement_QTCREATORBUG10366_1") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "enum test { TEST_1, TEST_2 };\n" - "\n" - "void f() {\n" - " enum test test;\n" - " @switch (test) {\n" - " }\n" - "}\n" - ) << _( - "enum test { TEST_1, TEST_2 };\n" - "\n" - "void f() {\n" - " enum test test;\n" - " switch (test) {\n" - " case TEST_1:\n" - " break;\n" - " case TEST_2:\n" - " break;\n" - " }\n" - "}\n" - ); - - // Same as above for enum class. - QTest::newRow("CompleteSwitchCaseStatement_QTCREATORBUG10366_1_enumClass") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "enum class test { TEST_1, TEST_2 };\n" - "\n" - "void f() {\n" - " enum test test;\n" - " @switch (test) {\n" - " }\n" - "}\n" - ) << _( - "enum class test { TEST_1, TEST_2 };\n" - "\n" - "void f() {\n" - " enum test test;\n" - " switch (test) {\n" - " case test::TEST_1:\n" - " break;\n" - " case test::TEST_2:\n" - " break;\n" - " }\n" - "}\n" - ); - - // Checks: Find the correct enum type despite there being a declaration with the same name. - QTest::newRow("CompleteSwitchCaseStatement_QTCREATORBUG10366_2") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "enum test1 { Wrong11, Wrong12 };\n" - "enum test { Right1, Right2 };\n" - "enum test2 { Wrong21, Wrong22 };\n" - "\n" - "int main() {\n" - " enum test test;\n" - " @switch (test) {\n" - " }\n" - "}\n" - ) << _( - "enum test1 { Wrong11, Wrong12 };\n" - "enum test { Right1, Right2 };\n" - "enum test2 { Wrong21, Wrong22 };\n" - "\n" - "int main() {\n" - " enum test test;\n" - " switch (test) {\n" - " case Right1:\n" - " break;\n" - " case Right2:\n" - " break;\n" - " }\n" - "}\n" - ); - - // Same as above for enum class. - QTest::newRow("CompleteSwitchCaseStatement_QTCREATORBUG10366_2_enumClass") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "enum class test1 { Wrong11, Wrong12 };\n" - "enum class test { Right1, Right2 };\n" - "enum class test2 { Wrong21, Wrong22 };\n" - "\n" - "int main() {\n" - " enum test test;\n" - " @switch (test) {\n" - " }\n" - "}\n" - ) << _( - "enum class test1 { Wrong11, Wrong12 };\n" - "enum class test { Right1, Right2 };\n" - "enum class test2 { Wrong21, Wrong22 };\n" - "\n" - "int main() {\n" - " enum test test;\n" - " switch (test) {\n" - " case test::Right1:\n" - " break;\n" - " case test::Right2:\n" - " break;\n" - " }\n" - "}\n" - ); - - // Checks: Do not crash on incomplete case statetement. - QTest::newRow("CompleteSwitchCaseStatement_doNotCrashOnIncompleteCase") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "enum E {};\n" - "void f(E o)\n" - "{\n" - " @switch (o)\n" - " {\n" - " case\n" - " }\n" - "}\n" - ) << _( - "" - ); - - // Same as above for enum class. - QTest::newRow("CompleteSwitchCaseStatement_doNotCrashOnIncompleteCase_enumClass") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "enum class E {};\n" - "void f(E o)\n" - "{\n" - " @switch (o)\n" - " {\n" - " case\n" - " }\n" - "}\n" - ) << _( - "" - ); - - // Checks: complete switch statement where enum is goes via a template type parameter - QTest::newRow("CompleteSwitchCaseStatement_QTCREATORBUG-24752") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "enum E {EA, EB};\n" - "template struct S {\n" - " static T theType() { return T(); }\n" - "};\n" - "int main() {\n" - " @switch (S::theType()) {\n" - " }\n" - "}\n" - ) << _( - "enum E {EA, EB};\n" - "template struct S {\n" - " static T theType() { return T(); }\n" - "};\n" - "int main() {\n" - " switch (S::theType()) {\n" - " case EA:\n" - " break;\n" - " case EB:\n" - " break;\n" - " }\n" - "}\n" - ); - - // Same as above for enum class. - QTest::newRow("CompleteSwitchCaseStatement_QTCREATORBUG-24752_enumClass") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "enum class E {A, B};\n" - "template struct S {\n" - " static T theType() { return T(); }\n" - "};\n" - "int main() {\n" - " @switch (S::theType()) {\n" - " }\n" - "}\n" - ) << _( - "enum class E {A, B};\n" - "template struct S {\n" - " static T theType() { return T(); }\n" - "};\n" - "int main() {\n" - " switch (S::theType()) {\n" - " case E::A:\n" - " break;\n" - " case E::B:\n" - " break;\n" - " }\n" - "}\n" - ); - - // Checks: Complete switch statement where enum is return type of a template function - // which is outside the scope of the return value. - // TODO: Type minimization. - QTest::newRow("CompleteSwitchCaseStatement_QTCREATORBUG-25998") - << CppQuickFixFactoryPtr(new CompleteSwitchCaseStatement) << _( - "template T enumCast(int value) { return static_cast(value); }\n" - "class Test {\n" - " enum class E { V1, V2 };" - " void func(int i) {\n" - " @switch (enumCast(i)) {\n" - " }\n" - " }\n" - "};\n" - ) << _( - "template T enumCast(int value) { return static_cast(value); }\n" - "class Test {\n" - " enum class E { V1, V2 };" - " void func(int i) {\n" - " switch (enumCast(i)) {\n" - " case Test::E::V1:\n" - " break;\n" - " case Test::E::V2:\n" - " break;\n" - " }\n" - " }\n" - "};\n" - ); - // Check: Just a basic test since the main functionality is tested in // cpppointerdeclarationformatter_test.cpp QTest::newRow("ReformatPointerDeclaration") diff --git a/src/plugins/cppeditor/quickfixes/cppquickfixes.cpp b/src/plugins/cppeditor/quickfixes/cppquickfixes.cpp index be988f39c2b..5c29d10713d 100644 --- a/src/plugins/cppeditor/quickfixes/cppquickfixes.cpp +++ b/src/plugins/cppeditor/quickfixes/cppquickfixes.cpp @@ -15,12 +15,12 @@ #include "../symbolfinder.h" #include "assigntolocalvariable.h" #include "bringidentifierintoscope.h" +#include "completeswitchstatement.h" #include "convertfromandtopointer.h" #include "cppcodegenerationquickfixes.h" #include "cppinsertvirtualmethods.h" #include "cppquickfixassistant.h" #include "cppquickfixhelpers.h" -#include "cppquickfixprojectsettings.h" #include "convertqt4connect.h" #include "convertstringliteral.h" #include "createdeclarationfromuse.h" @@ -693,183 +693,6 @@ void ReformatPointerDeclaration::doMatch(const CppQuickFixInterface &interface, namespace { -class CaseStatementCollector : public ASTVisitor -{ -public: - CaseStatementCollector(Document::Ptr document, const Snapshot &snapshot, - Scope *scope) - : ASTVisitor(document->translationUnit()), - document(document), - scope(scope) - { - typeOfExpression.init(document, snapshot); - } - - QStringList operator ()(AST *ast) - { - values.clear(); - foundCaseStatementLevel = false; - accept(ast); - return values; - } - - bool preVisit(AST *ast) override { - if (CaseStatementAST *cs = ast->asCaseStatement()) { - foundCaseStatementLevel = true; - if (ExpressionAST *csExpression = cs->expression) { - if (ExpressionAST *expression = csExpression->asIdExpression()) { - QList candidates = typeOfExpression(expression, document, scope); - if (!candidates.isEmpty() && candidates.first().declaration()) { - Symbol *decl = candidates.first().declaration(); - values << prettyPrint.prettyName(LookupContext::fullyQualifiedName(decl)); - } - } - } - return true; - } else if (foundCaseStatementLevel) { - return false; - } - return true; - } - - Overview prettyPrint; - bool foundCaseStatementLevel = false; - QStringList values; - TypeOfExpression typeOfExpression; - Document::Ptr document; - Scope *scope; -}; - -class CompleteSwitchCaseStatementOp: public CppQuickFixOperation -{ -public: - CompleteSwitchCaseStatementOp(const CppQuickFixInterface &interface, - int priority, CompoundStatementAST *compoundStatement, const QStringList &values) - : CppQuickFixOperation(interface, priority) - , compoundStatement(compoundStatement) - , values(values) - { - setDescription(Tr::tr("Complete Switch Statement")); - } - - void perform() override - { - CppRefactoringChanges refactoring(snapshot()); - CppRefactoringFilePtr currentFile = refactoring.cppFile(filePath()); - - ChangeSet changes; - int start = currentFile->endOf(compoundStatement->lbrace_token); - changes.insert(start, QLatin1String("\ncase ") - + values.join(QLatin1String(":\nbreak;\ncase ")) - + QLatin1String(":\nbreak;")); - currentFile->setChangeSet(changes); - currentFile->apply(); - } - - CompoundStatementAST *compoundStatement; - QStringList values; -}; - -static Enum *findEnum(const QList &results, const LookupContext &ctxt) -{ - for (const LookupItem &result : results) { - const FullySpecifiedType fst = result.type(); - - Type *type = result.declaration() ? result.declaration()->type().type() - : fst.type(); - - if (!type) - continue; - if (Enum *e = type->asEnumType()) - return e; - if (const NamedType *namedType = type->asNamedType()) { - if (ClassOrNamespace *con = ctxt.lookupType(namedType->name(), result.scope())) { - QList enums = con->unscopedEnums(); - const QList symbols = con->symbols(); - for (Symbol * const s : symbols) { - if (const auto e = s->asEnum()) - enums << e; - } - const Name *referenceName = namedType->name(); - if (const QualifiedNameId *qualifiedName = referenceName->asQualifiedNameId()) - referenceName = qualifiedName->name(); - for (Enum *e : std::as_const(enums)) { - if (const Name *candidateName = e->name()) { - if (candidateName->match(referenceName)) - return e; - } - } - } - } - } - - return nullptr; -} - -Enum *conditionEnum(const CppQuickFixInterface &interface, SwitchStatementAST *statement) -{ - Block *block = statement->symbol; - Scope *scope = interface.semanticInfo().doc->scopeAt(block->line(), block->column()); - TypeOfExpression typeOfExpression; - typeOfExpression.setExpandTemplates(true); - typeOfExpression.init(interface.semanticInfo().doc, interface.snapshot()); - const QList results = typeOfExpression(statement->condition, - interface.semanticInfo().doc, - scope); - - return findEnum(results, typeOfExpression.context()); -} - -} // anonymous namespace - -void CompleteSwitchCaseStatement::doMatch(const CppQuickFixInterface &interface, - QuickFixOperations &result) -{ - const QList &path = interface.path(); - - if (path.isEmpty()) - return; - - // look for switch statement - for (int depth = path.size() - 1; depth >= 0; --depth) { - AST *ast = path.at(depth); - SwitchStatementAST *switchStatement = ast->asSwitchStatement(); - if (switchStatement) { - if (!switchStatement->statement || !switchStatement->symbol) - return; - CompoundStatementAST *compoundStatement = switchStatement->statement->asCompoundStatement(); - if (!compoundStatement) // we ignore pathologic case "switch (t) case A: ;" - return; - // look if the condition's type is an enum - if (Enum *e = conditionEnum(interface, switchStatement)) { - // check the possible enum values - QStringList values; - Overview prettyPrint; - for (int i = 0; i < e->memberCount(); ++i) { - if (Declaration *decl = e->memberAt(i)->asDeclaration()) - values << prettyPrint.prettyName(LookupContext::fullyQualifiedName(decl)); - } - // Get the used values - Block *block = switchStatement->symbol; - CaseStatementCollector caseValues(interface.semanticInfo().doc, interface.snapshot(), - interface.semanticInfo().doc->scopeAt(block->line(), block->column())); - const QStringList usedValues = caseValues(switchStatement); - // save the values that would be added - for (const QString &usedValue : usedValues) - values.removeAll(usedValue); - if (!values.isEmpty()) - result << new CompleteSwitchCaseStatementOp(interface, depth, - compoundStatement, values); - return; - } - - return; - } - } -} - -namespace { - class ApplyDeclDefLinkOperation : public CppQuickFixOperation { public: @@ -1060,8 +883,6 @@ void createCppQuickFixes() new RearrangeParamDeclarationList; new ReformatPointerDeclaration; - new CompleteSwitchCaseStatement; - new ApplyDeclDefLinkChanges; registerInsertVirtualMethodsQuickfix(); @@ -1081,6 +902,7 @@ void createCppQuickFixes() registerExtractLiteralAsParameterQuickfix(); registerConvertFromAndToPointerQuickfix(); registerAssignToLocalVariableQuickfix(); + registerCompleteSwitchStatementQuickfix(); new ExtraRefactoringOperations; diff --git a/src/plugins/cppeditor/quickfixes/cppquickfixes.h b/src/plugins/cppeditor/quickfixes/cppquickfixes.h index c85bc0625df..3e7ba78bc92 100644 --- a/src/plugins/cppeditor/quickfixes/cppquickfixes.h +++ b/src/plugins/cppeditor/quickfixes/cppquickfixes.h @@ -113,18 +113,6 @@ public: void doMatch(const CppQuickFixInterface &interface, QuickFixOperations &result) override; }; -/*! - Adds missing case statements for "switch (enumVariable)" - */ -class CompleteSwitchCaseStatement: public CppQuickFixFactory -{ -public: - CompleteSwitchCaseStatement() { setClangdReplacement({12}); } - -private: - void doMatch(const CppQuickFixInterface &interface, QuickFixOperations &result) override; -}; - /*! Applies function signature changes */