From 365e02e73ea9e6177ff5fb6de73e4187eded8afc Mon Sep 17 00:00:00 2001 From: Mateusz Janek Date: Thu, 25 Jan 2018 07:28:53 +0100 Subject: source_group: Fix TREE argument parsing Fixes: #17581 --- Source/cmSourceGroupCommand.cxx | 224 ++++++++++++++++++----------- Source/cmSourceGroupCommand.h | 23 ++- Tests/SourceGroups/CMakeLists.txt | 10 +- Tests/SourceGroups/main.c | 7 +- Tests/SourceGroups/tree_empty_prefix_bar.c | 4 + Tests/SourceGroups/tree_empty_prefix_foo.c | 4 + 6 files changed, 181 insertions(+), 91 deletions(-) create mode 100644 Tests/SourceGroups/tree_empty_prefix_bar.c create mode 100644 Tests/SourceGroups/tree_empty_prefix_foo.c diff --git a/Source/cmSourceGroupCommand.cxx b/Source/cmSourceGroupCommand.cxx index 69983a8..87ecc56 100644 --- a/Source/cmSourceGroupCommand.cxx +++ b/Source/cmSourceGroupCommand.cxx @@ -2,19 +2,21 @@ file Copyright.txt or https://cmake.org/licensing for details. */ #include "cmSourceGroupCommand.h" +#include #include -#include #include +#include #include "cmMakefile.h" #include "cmSourceGroup.h" #include "cmSystemTools.h" namespace { -const size_t RootIndex = 1; -const size_t FilesWithoutPrefixKeywordIndex = 2; -const size_t FilesWithPrefixKeywordIndex = 4; -const size_t PrefixKeywordIndex = 2; +const std::string kTreeOptionName = "TREE"; +const std::string kPrefixOptionName = "PREFIX"; +const std::string kFilesOptionName = "FILES"; +const std::string kRegexOptionName = "REGULAR_EXPRESSION"; +const std::string kSourceGroupOptionName = ""; std::vector tokenizePath(const std::string& path) { @@ -71,14 +73,13 @@ std::string prepareFilePathForTree(const std::string& path, } std::vector prepareFilesPathsForTree( - std::vector::const_iterator begin, - std::vector::const_iterator end, + const std::vector& filesPaths, const std::string& currentSourceDir) { std::vector prepared; - for (; begin != end; ++begin) { - prepared.push_back(prepareFilePathForTree(*begin, currentSourceDir)); + for (auto const& filePath : filesPaths) { + prepared.push_back(prepareFilePathForTree(filePath, currentSourceDir)); } return prepared; @@ -121,6 +122,57 @@ bool addFilesToItsSourceGroups(const std::string& root, class cmExecutionStatus; // cmSourceGroupCommand +cmSourceGroupCommand::ExpectedOptions +cmSourceGroupCommand::getExpectedOptions() const +{ + ExpectedOptions options; + + options.push_back(kTreeOptionName); + options.push_back(kPrefixOptionName); + options.push_back(kFilesOptionName); + options.push_back(kRegexOptionName); + + return options; +} + +bool cmSourceGroupCommand::isExpectedOption( + const std::string& argument, const ExpectedOptions& expectedOptions) +{ + return std::find(expectedOptions.begin(), expectedOptions.end(), argument) != + expectedOptions.end(); +} + +void cmSourceGroupCommand::parseArguments( + const std::vector& args, + cmSourceGroupCommand::ParsedArguments& parsedArguments) +{ + const ExpectedOptions expectedOptions = getExpectedOptions(); + size_t i = 0; + + // at this point we know that args vector is not empty + + // if first argument is not one of expected options it's source group name + if (!isExpectedOption(args[0], expectedOptions)) { + // get source group name and go to next argument + parsedArguments[kSourceGroupOptionName].push_back(args[0]); + ++i; + } + + for (; i < args.size();) { + // get current option and increment index to go to next argument + const std::string& currentOption = args[i++]; + + // create current option entry in parsed arguments + std::vector& currentOptionArguments = + parsedArguments[currentOption]; + + // collect option arguments while we won't find another expected option + while (i < args.size() && !isExpectedOption(args[i], expectedOptions)) { + currentOptionArguments.push_back(args[i++]); + } + } +} + bool cmSourceGroupCommand::InitialPass(std::vector const& args, cmExecutionStatus&) { @@ -129,114 +181,98 @@ bool cmSourceGroupCommand::InitialPass(std::vector const& args, return false; } - if (args[0] == "TREE") { - std::string error; + // If only two arguments are given, the pre-1.8 version of the + // command is being invoked. + if (args.size() == 2 && args[1] != "FILES") { + cmSourceGroup* sg = this->Makefile->GetOrCreateSourceGroup(args[0]); - if (!processTree(args, error)) { - this->SetError(error); + if (!sg) { + this->SetError("Could not create or find source group"); return false; } + sg->SetGroupRegex(args[1].c_str()); return true; } - cmSourceGroup* sg = this->Makefile->GetOrCreateSourceGroup(args[0]); + ParsedArguments parsedArguments; + std::string errorMsg; - if (!sg) { - this->SetError("Could not create or find source group"); + parseArguments(args, parsedArguments); + + if (!checkArgumentsPreconditions(parsedArguments, errorMsg)) { return false; } - // If only two arguments are given, the pre-1.8 version of the - // command is being invoked. - if (args.size() == 2 && args[1] != "FILES") { - sg->SetGroupRegex(args[1].c_str()); - return true; - } - // Process arguments. - bool doingFiles = false; - for (unsigned int i = 1; i < args.size(); ++i) { - if (args[i] == "REGULAR_EXPRESSION") { - // Next argument must specify the regex. - if (i + 1 < args.size()) { - ++i; - sg->SetGroupRegex(args[i].c_str()); - } else { - this->SetError("REGULAR_EXPRESSION argument given without a regex."); - return false; - } - doingFiles = false; - } else if (args[i] == "FILES") { - // Next arguments will specify files. - doingFiles = true; - } else if (doingFiles) { - // Convert name to full path and add to the group's list. - std::string src = args[i]; + if (parsedArguments.find(kTreeOptionName) != parsedArguments.end()) { + if (!processTree(parsedArguments, errorMsg)) { + this->SetError(errorMsg); + return false; + } + } else { + if (parsedArguments.find(kSourceGroupOptionName) == + parsedArguments.end()) { + this->SetError("Missing source group name."); + return false; + } + + cmSourceGroup* sg = this->Makefile->GetOrCreateSourceGroup(args[0]); + + if (!sg) { + this->SetError("Could not create or find source group"); + return false; + } + + // handle regex + if (parsedArguments.find(kRegexOptionName) != parsedArguments.end()) { + const std::string& sgRegex = parsedArguments[kRegexOptionName].front(); + sg->SetGroupRegex(sgRegex.c_str()); + } + + // handle files + const std::vector& filesArguments = + parsedArguments[kFilesOptionName]; + for (auto const& filesArg : filesArguments) { + std::string src = filesArg; if (!cmSystemTools::FileIsFullPath(src.c_str())) { src = this->Makefile->GetCurrentSourceDirectory(); src += "/"; - src += args[i]; + src += filesArg; } src = cmSystemTools::CollapseFullPath(src); sg->AddGroupFile(src); - } else { - std::ostringstream err; - err << "Unknown argument \"" << args[i] << "\". " - << "Perhaps the FILES keyword is missing.\n"; - this->SetError(err.str()); - return false; } } return true; } -bool cmSourceGroupCommand::checkTreeArgumentsPreconditions( - const std::vector& args, std::string& errorMsg) const +bool cmSourceGroupCommand::checkArgumentsPreconditions( + const ParsedArguments& parsedArguments, std::string& errorMsg) const { - if (args.size() == 1) { - errorMsg = "TREE argument given without a root."; - return false; - } - - if (args.size() < 3) { - errorMsg = "Missing FILES arguments."; - return false; - } - - if (args[FilesWithoutPrefixKeywordIndex] != "FILES" && - args[PrefixKeywordIndex] != "PREFIX") { - errorMsg = "Unknown argument \"" + args[2] + - "\". Perhaps the FILES keyword is missing.\n"; - return false; - } - - if (args[PrefixKeywordIndex] == "PREFIX" && - (args.size() < 5 || args[FilesWithPrefixKeywordIndex] != "FILES")) { - errorMsg = "Missing FILES arguments."; + if (!checkSingleParameterArgumentPreconditions(kPrefixOptionName, + parsedArguments, errorMsg) || + !checkSingleParameterArgumentPreconditions(kTreeOptionName, + parsedArguments, errorMsg) || + !checkSingleParameterArgumentPreconditions(kRegexOptionName, + parsedArguments, errorMsg)) { return false; } return true; } -bool cmSourceGroupCommand::processTree(const std::vector& args, +bool cmSourceGroupCommand::processTree(ParsedArguments& parsedArguments, std::string& errorMsg) { - if (!checkTreeArgumentsPreconditions(args, errorMsg)) { - return false; - } - - const std::string root = cmSystemTools::CollapseFullPath(args[RootIndex]); - std::string prefix; - size_t filesBegin = FilesWithoutPrefixKeywordIndex + 1; - if (args[PrefixKeywordIndex] == "PREFIX") { - prefix = args[PrefixKeywordIndex + 1]; - filesBegin = FilesWithPrefixKeywordIndex + 1; - } + const std::string root = + cmSystemTools::CollapseFullPath(parsedArguments[kTreeOptionName].front()); + std::string prefix = parsedArguments[kPrefixOptionName].empty() + ? "" + : parsedArguments[kPrefixOptionName].front(); const std::vector filesVector = - prepareFilesPathsForTree(args.begin() + filesBegin, args.end(), + prepareFilesPathsForTree(parsedArguments[kFilesOptionName], this->Makefile->GetCurrentSourceDirectory()); if (!rootIsPrefix(root, filesVector, errorMsg)) { @@ -253,3 +289,25 @@ bool cmSourceGroupCommand::processTree(const std::vector& args, return true; } + +bool cmSourceGroupCommand::checkSingleParameterArgumentPreconditions( + const std::string& argument, const ParsedArguments& parsedArguments, + std::string& errorMsg) const +{ + ParsedArguments::const_iterator foundArgument = + parsedArguments.find(argument); + if (foundArgument != parsedArguments.end()) { + const std::vector& optionArguments = foundArgument->second; + + if (optionArguments.empty()) { + errorMsg = argument + " argument given without an argument."; + return false; + } + if (optionArguments.size() > 1) { + errorMsg = "too many arguments passed to " + argument + "."; + return false; + } + } + + return true; +} diff --git a/Source/cmSourceGroupCommand.h b/Source/cmSourceGroupCommand.h index ed02ca5..ec5ad32 100644 --- a/Source/cmSourceGroupCommand.h +++ b/Source/cmSourceGroupCommand.h @@ -5,6 +5,7 @@ #include "cmConfigure.h" // IWYU pragma: keep +#include #include #include @@ -34,10 +35,24 @@ public: cmExecutionStatus& status) override; private: - bool processTree(const std::vector& args, - std::string& errorMsg); - bool checkTreeArgumentsPreconditions(const std::vector& args, - std::string& errorMsg) const; + typedef std::map> ParsedArguments; + typedef std::vector ExpectedOptions; + + ExpectedOptions getExpectedOptions() const; + + bool isExpectedOption(const std::string& argument, + const ExpectedOptions& expectedOptions); + + void parseArguments(const std::vector& args, + cmSourceGroupCommand::ParsedArguments& parsedArguments); + + bool processTree(ParsedArguments& parsedArguments, std::string& errorMsg); + + bool checkArgumentsPreconditions(const ParsedArguments& parsedArguments, + std::string& errorMsg) const; + bool checkSingleParameterArgumentPreconditions( + const std::string& argument, const ParsedArguments& parsedArguments, + std::string& errorMsg) const; }; #endif diff --git a/Tests/SourceGroups/CMakeLists.txt b/Tests/SourceGroups/CMakeLists.txt index 4e4a030..813774d 100644 --- a/Tests/SourceGroups/CMakeLists.txt +++ b/Tests/SourceGroups/CMakeLists.txt @@ -39,9 +39,15 @@ set(tree_files_without_prefix ${root}/sub1/tree_bar.c set(tree_files_with_prefix ${root}/tree_prefix_foo.c tree_prefix_bar.c) +set(tree_files_with_empty_prefix ${root}/tree_empty_prefix_foo.c + tree_empty_prefix_bar.c) + source_group(TREE ${root} FILES ${tree_files_without_prefix}) -source_group(TREE ${root} PREFIX tree_root/subgroup FILES ${tree_files_with_prefix}) +source_group(FILES ${tree_files_with_prefix} PREFIX tree_root/subgroup TREE ${root}) + +source_group(PREFIX "" FILES ${tree_files_with_empty_prefix} TREE ${root}) add_executable(SourceGroups main.c bar.c foo.c sub1/foo.c sub1/foobar.c baz.c - ${tree_files_with_prefix} ${tree_files_without_prefix} README.txt) + ${tree_files_with_prefix} ${tree_files_without_prefix} + ${tree_files_with_empty_prefix} README.txt) diff --git a/Tests/SourceGroups/main.c b/Tests/SourceGroups/main.c index 4d84cf2..87225f5 100644 --- a/Tests/SourceGroups/main.c +++ b/Tests/SourceGroups/main.c @@ -7,6 +7,8 @@ extern int barbar(void); extern int baz(void); extern int tree_prefix_foo(void); extern int tree_prefix_bar(void); +extern int tree_empty_prefix_foo(void); +extern int tree_empty_prefix_bar(void); extern int tree_bar(void); extern int tree_foobar(void); extern int tree_baz(void); @@ -17,8 +19,9 @@ int main() foobar(), barbar(), baz()); printf("tree_prefix_foo: %d tree_prefix_bar: %d tree_bar: %d tree_foobar: " - "%d tree_baz: %d\n", + "%d tree_baz: %d tree_empty_prefix_foo: %d " + "tree_empty_prefix_bar: %d\n", tree_prefix_foo(), tree_prefix_bar(), tree_bar(), tree_foobar(), - tree_baz()); + tree_baz(), tree_empty_prefix_foo(), tree_empty_prefix_bar()); return 0; } diff --git a/Tests/SourceGroups/tree_empty_prefix_bar.c b/Tests/SourceGroups/tree_empty_prefix_bar.c new file mode 100644 index 0000000..d974c6e --- /dev/null +++ b/Tests/SourceGroups/tree_empty_prefix_bar.c @@ -0,0 +1,4 @@ +int tree_empty_prefix_bar(void) +{ + return 66; +} diff --git a/Tests/SourceGroups/tree_empty_prefix_foo.c b/Tests/SourceGroups/tree_empty_prefix_foo.c new file mode 100644 index 0000000..277cbd4 --- /dev/null +++ b/Tests/SourceGroups/tree_empty_prefix_foo.c @@ -0,0 +1,4 @@ +int tree_empty_prefix_foo(void) +{ + return 6; +} -- cgit v0.12