From 33d2d736ea4f52725c0714064f5380277181e9a2 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Fri, 5 Feb 2021 13:12:41 +0100 Subject: [PATCH] CppEditor: Do not add redundant access specifier ... when inserting a member function declaration for a definition. Fixes: QTCREATORBUG-5591 Change-Id: Ie85b435a1595832d0085abdcc0a4187d939820e0 Reviewed-by: Christian Stenger --- src/plugins/cppeditor/cppquickfix_test.cpp | 5 ++++- src/plugins/cppeditor/cppquickfixes.cpp | 6 ++--- .../cpptools/insertionpointlocator.cpp | 22 ++++++++++++------- src/plugins/cpptools/insertionpointlocator.h | 22 +++++++++++++------ 4 files changed, 36 insertions(+), 19 deletions(-) diff --git a/src/plugins/cppeditor/cppquickfix_test.cpp b/src/plugins/cppeditor/cppquickfix_test.cpp index 7bfdf5b7ba5..5ba0101b399 100644 --- a/src/plugins/cppeditor/cppquickfix_test.cpp +++ b/src/plugins/cppeditor/cppquickfix_test.cpp @@ -4707,6 +4707,9 @@ void insertToSectionDeclFromDef(const QByteArray §ion, int sectionIndex) QByteArray original; QByteArray expected; + QByteArray sectionString = section + ":\n"; + if (sectionIndex == 4) + sectionString.clear(); // Header File original = @@ -4716,7 +4719,7 @@ void insertToSectionDeclFromDef(const QByteArray §ion, int sectionIndex) expected = "class Foo\n" "{\n" - + section + ":\n" + + + sectionString + " Foo();\n" "@};\n"; testDocuments << QuickFixTestDocument::create("file.h", original, expected); diff --git a/src/plugins/cppeditor/cppquickfixes.cpp b/src/plugins/cppeditor/cppquickfixes.cpp index 6dda12c1f51..f0d205215af 100644 --- a/src/plugins/cppeditor/cppquickfixes.cpp +++ b/src/plugins/cppeditor/cppquickfixes.cpp @@ -3962,9 +3962,9 @@ protected: const auto insertionPoint = m_headerInsertionPoints.find(spec); if (insertionPoint != m_headerInsertionPoints.end()) return *insertionPoint; - const InsertionLocation loc = m_locator.methodDeclarationInClass(m_headerFile->fileName(), - m_class, - spec); + const InsertionLocation loc = m_locator.methodDeclarationInClass( + m_headerFile->fileName(), m_class, spec, + InsertionPointLocator::ForceAccessSpec::Yes); m_headerInsertionPoints.insert(spec, loc); return loc; } diff --git a/src/plugins/cpptools/insertionpointlocator.cpp b/src/plugins/cpptools/insertionpointlocator.cpp index 5ccfda63195..d258937cb7d 100644 --- a/src/plugins/cpptools/insertionpointlocator.cpp +++ b/src/plugins/cpptools/insertionpointlocator.cpp @@ -118,6 +118,7 @@ private: void findMatch(const QList &ranges, InsertionPointLocator::AccessSpec xsSpec, InsertionPointLocator::Position positionInAccessSpec, + InsertionPointLocator::ForceAccessSpec forceAccessSpec, unsigned &beforeToken, bool &needsLeadingEmptyLine, bool &needsPrefix, @@ -128,8 +129,10 @@ void findMatch(const QList &ranges, const bool atEnd = positionInAccessSpec == InsertionPointLocator::AccessSpecEnd; needsLeadingEmptyLine = false; - // try an exact match, and ignore the first (default) access spec: - for (int i = lastIndex; i > 0; --i) { + // Try an exact match. Ignore the default access spec unless there is no explicit one. + const int firstIndex = ranges.size() == 1 + && forceAccessSpec == InsertionPointLocator::ForceAccessSpec::No ? 0 : 1; + for (int i = lastIndex; i >= firstIndex; --i) { const AccessRange &range = ranges.at(i); if (range.xsSpec == xsSpec) { beforeToken = atEnd ? range.end : range.beforeStart; @@ -270,23 +273,25 @@ InsertionPointLocator::InsertionPointLocator(const CppRefactoringChanges &refact InsertionLocation InsertionPointLocator::methodDeclarationInClass( const QString &fileName, const Class *clazz, - AccessSpec xsSpec) const + AccessSpec xsSpec, + ForceAccessSpec forceAccessSpec) const { const Document::Ptr doc = m_refactoringChanges.file(fileName)->cppDocument(); if (doc) { FindInClass find(doc->translationUnit(), clazz); ClassSpecifierAST *classAST = find(); - return methodDeclarationInClass(doc->translationUnit(), classAST, xsSpec); + return methodDeclarationInClass(doc->translationUnit(), classAST, xsSpec, AccessSpecEnd, + forceAccessSpec); } else { return InsertionLocation(); } } -InsertionLocation InsertionPointLocator::methodDeclarationInClass( - const TranslationUnit *tu, +InsertionLocation InsertionPointLocator::methodDeclarationInClass(const TranslationUnit *tu, const ClassSpecifierAST *clazz, InsertionPointLocator::AccessSpec xsSpec, - Position pos) const + Position pos, + ForceAccessSpec forceAccessSpec) const { if (!clazz) return {}; @@ -302,7 +307,8 @@ InsertionLocation InsertionPointLocator::methodDeclarationInClass( bool needsLeadingEmptyLine = false; bool needsPrefix = false; bool needsSuffix = false; - findMatch(ranges, xsSpec, pos, beforeToken, needsLeadingEmptyLine, needsPrefix, needsSuffix); + findMatch(ranges, xsSpec, pos, forceAccessSpec, beforeToken, needsLeadingEmptyLine, + needsPrefix, needsSuffix); int line = 0, column = 0; if (pos == InsertionPointLocator::AccessSpecEnd) diff --git a/src/plugins/cpptools/insertionpointlocator.h b/src/plugins/cpptools/insertionpointlocator.h index 1d79a7000fb..c20c45151bc 100644 --- a/src/plugins/cpptools/insertionpointlocator.h +++ b/src/plugins/cpptools/insertionpointlocator.h @@ -91,17 +91,25 @@ public: AccessSpecEnd, }; + enum class ForceAccessSpec { Yes, No }; + public: explicit InsertionPointLocator(const CppRefactoringChanges &refactoringChanges); - InsertionLocation methodDeclarationInClass(const QString &fileName, - const CPlusPlus::Class *clazz, - AccessSpec xsSpec) const; + InsertionLocation methodDeclarationInClass( + const QString &fileName, + const CPlusPlus::Class *clazz, + AccessSpec xsSpec, + ForceAccessSpec forceAccessSpec = ForceAccessSpec::No + ) const; - InsertionLocation methodDeclarationInClass(const CPlusPlus::TranslationUnit *tu, - const CPlusPlus::ClassSpecifierAST *clazz, - AccessSpec xsSpec, - Position positionInAccessSpec = AccessSpecEnd) const; + InsertionLocation methodDeclarationInClass( + const CPlusPlus::TranslationUnit *tu, + const CPlusPlus::ClassSpecifierAST *clazz, + AccessSpec xsSpec, + Position positionInAccessSpec = AccessSpecEnd, + ForceAccessSpec forceAccessSpec = ForceAccessSpec::No + ) const; InsertionLocation constructorDeclarationInClass(const CPlusPlus::TranslationUnit *tu, const CPlusPlus::ClassSpecifierAST *clazz,