diff --git a/libs/parser/DefBlockSyntaxParser.h b/libs/parser/DefBlockSyntaxParser.h index 2d8a532ff3..4edac97773 100644 --- a/libs/parser/DefBlockSyntaxParser.h +++ b/libs/parser/DefBlockSyntaxParser.h @@ -2,6 +2,7 @@ #include #include +#include #include "string/tokeniser.h" #include "string/join.h" @@ -118,6 +119,12 @@ class DefWhitespaceSyntax : assert(token.type == DefSyntaxToken::Type::Whitespace); } + // Returns the number of line breaks this whitespace contains + std::size_t getNumberOfLineBreaks() + { + return std::count_if(_token.value.begin(), _token.value.end(), [](char c) { return c == '\n'; }); + } + std::string getString() const override { return _token.value; diff --git a/radiantcore/decl/DeclarationManager.cpp b/radiantcore/decl/DeclarationManager.cpp index 0b5a2b9f98..9cf0fbcbae 100644 --- a/radiantcore/decl/DeclarationManager.cpp +++ b/radiantcore/decl/DeclarationManager.cpp @@ -325,6 +325,79 @@ void DeclarationManager::removeDeclaration(Type type, const std::string& name) }); } +namespace +{ + +void removeDeclarationFromSyntaxTree(const parser::DefSyntaxTree::Ptr& syntaxTree, const std::string& declName) +{ + // Remove the declaration from the tree + std::vector nodesToRemove; + + const auto& childNodes = syntaxTree->getRoot()->getChildren(); + for (int i = 0; i < childNodes.size(); ++i) + { + const auto& node = childNodes.at(i); + + if (node->getType() != parser::DefSyntaxNode::Type::DeclBlock) continue; + + auto blockNode = std::static_pointer_cast(node); + + if (blockNode->getName() && blockNode->getName()->getToken().value == declName) + { + nodesToRemove.push_back(blockNode); + + parser::DefSyntaxNode::Ptr encounteredWhitespace; + + // Try to locate comment nodes preceding this block + for (int p = i - 1; p >= 0; --p) + { + const auto& predecessor = childNodes.at(p); + if (predecessor->getType() == parser::DefSyntaxNode::Type::Whitespace) + { + // Stop at the first whitespace token that contains more than one line break + auto whitespace = std::static_pointer_cast(predecessor); + + // stop searching at the first empty line + if (whitespace->getNumberOfLineBreaks() > 1) break; + + // This is just a single line break (or none), remember to remove it when we encounter a comment + encounteredWhitespace = predecessor; + } + else if (predecessor->getType() == parser::DefSyntaxNode::Type::Comment) + { + nodesToRemove.push_back(predecessor); + + // Remove any whitespace that stood between this comment and the block + if (encounteredWhitespace) + { + nodesToRemove.push_back(encounteredWhitespace); + encounteredWhitespace.reset(); + } + + continue; + } + else // stop at all other node types + { + break; + } + } + } + } + + if (nodesToRemove.empty()) + { + rWarning() << "Could not locate the decl block " << declName << std::endl; + return; + } + + for (const auto& node : nodesToRemove) + { + syntaxTree->getRoot()->removeChildNode(node); + } +} + +} + void DeclarationManager::removeDeclarationFromFile(const IDeclaration::Ptr& decl) { const auto& syntax = decl->getBlockSyntax(); @@ -361,33 +434,9 @@ void DeclarationManager::removeDeclarationFromFile(const IDeclaration::Ptr& decl auto syntaxTree = parser.parse(); inheritStream.close(); - // Remove the declaration from the tree - std::vector nodesToRemove; - - for (const auto& node : syntaxTree->getRoot()->getChildren()) - { - if (node->getType() != parser::DefSyntaxNode::Type::DeclBlock) continue; - - auto blockNode = std::static_pointer_cast(node); - - if (blockNode->getName() && blockNode->getName()->getToken().value == decl->getDeclName()) - { - nodesToRemove.push_back(blockNode); - } - } - - if (nodesToRemove.empty()) - { - rWarning() << "Could not locate the decl block " << decl->getDeclName() << " in the file " << fullPath << std::endl; - return; - } - - for (const auto& node : nodesToRemove) - { - syntaxTree->getRoot()->removeChildNode(node); - } + removeDeclarationFromSyntaxTree(syntaxTree, decl->getDeclName()); - // Export the syntax tree with the missing decl block + // Export the modified syntax tree stream << syntaxTree->getString(); tempStream.closeAndReplaceTargetFile(); diff --git a/test/DeclManager.cpp b/test/DeclManager.cpp index d07ec05528..ec78d0a528 100644 --- a/test/DeclManager.cpp +++ b/test/DeclManager.cpp @@ -903,6 +903,46 @@ TEST_F(DeclManagerTest, RemoveDeclarationRemovesLeadingMultilineComment) EXPECT_TRUE(algorithm::fileContainsText(fullPath, "// Some comments after decl/removal/2")); } +// Multiple comments preceding the decl with minor whitespace in between +TEST_F(DeclManagerTest, RemoveDeclarationRemovesLeadingMixedComment) +{ + GlobalDeclarationManager().registerDeclType("testdecl", std::make_shared()); + GlobalDeclarationManager().registerDeclFolder(decl::Type::TestDecl, TEST_DECL_FOLDER, ".decl"); + + // Create a backup copy of the decl file we're going to manipulate + auto fullPath = _context.getTestProjectPath() + "testdecls/removal_tests.decl"; + BackupCopy backup(fullPath); + + // decl/removal/3a has leading multiline comment, and a trailing single line comment + EXPECT_TRUE(algorithm::fileContainsText(fullPath, R"(// Multiple comment lines before decl/removal/a3 +/* mixed style comments too */ + // multiple comment with some whitespace at the start of each line + // multiple comment with some whitespace at the start of each line)")); + + GlobalDeclarationManager().removeDeclaration(decl::Type::TestDecl, "decl/removal/3a"); + + // Leading comment should be gone, trailing comment should stay + EXPECT_FALSE(algorithm::fileContainsText(fullPath, R"(decl/removal/3a +{ + diffusemap textures/removal/a3 +})")); + EXPECT_FALSE(algorithm::fileContainsText(fullPath, R"(// Multiple comment lines before decl/removal/a3 +/* mixed style comments too */ + // multiple comment with some whitespace at the start of each line + // multiple comment with some whitespace at the start of each line)")); + + // Safety check that nothing else got removed (including whitespace) + EXPECT_TRUE(algorithm::fileContainsText(fullPath, R"( + +// Comment with a line before the start of the decl decl/removal/5)")); + EXPECT_TRUE(algorithm::fileContainsText(fullPath, R"(TestDecl decl/removal/3 +{ + diffusemap textures/removal/3 +} + +)")); +} + // There's an evil misleading commented out declaration decl/removal/9 in the file // right before the actual, uncommented one TEST_F(DeclManagerTest, RemoveDeclarationIgnoresCommentedContent) diff --git a/test/resources/tdm/testdecls/removal_tests.decl b/test/resources/tdm/testdecls/removal_tests.decl index 6e2705e19b..f1264ed0a4 100644 --- a/test/resources/tdm/testdecls/removal_tests.decl +++ b/test/resources/tdm/testdecls/removal_tests.decl @@ -29,6 +29,15 @@ TestDecl decl/removal/3 diffusemap textures/removal/3 } +// Multiple comment lines before decl/removal/a3 +/* mixed style comments too */ + // multiple comment with some whitespace at the start of each line + // multiple comment with some whitespace at the start of each line +testDecl decl/removal/3a +{ + diffusemap textures/removal/a3 +} + // Comment with a line before the start of the decl decl/removal/5 testdecl decl/removal/5 { // a comment in the same line as the opening brace