diff --git a/include/openspace/documentation/verifier.h b/include/openspace/documentation/verifier.h index 401533f5e2..dd263a6ea5 100644 --- a/include/openspace/documentation/verifier.h +++ b/include/openspace/documentation/verifier.h @@ -168,6 +168,21 @@ private: bool _mustBeNotEmpty = false; }; +/** + * A Verifier that checks whether a given string is a valid identifier, meaning that is + * does not contain any whitespaces or dots + */ +struct IdentifierVerifier : public StringVerifier { + IdentifierVerifier(); + + TestResult operator()(const ghoul::Dictionary& dict, + const std::string& key) const override; + + std::string documentation() const override; + + std::string type() const override; +}; + /** * A Verifier that checks whether a given key inside a ghoul::Dictionary is a string and * refers to an existing file on disk. diff --git a/modules/base/lightsource/scenegraphlightsource.cpp b/modules/base/lightsource/scenegraphlightsource.cpp index 53837b21c5..1844919d23 100644 --- a/modules/base/lightsource/scenegraphlightsource.cpp +++ b/modules/base/lightsource/scenegraphlightsource.cpp @@ -51,7 +51,7 @@ namespace { std::optional intensity; // [[codegen::verbatim(NodeInfo.description)]] - std::string node; + std::string node [[codegen::identifier()]]; }; #include "scenegraphlightsource_codegen.cpp" } // namespace diff --git a/modules/base/rendering/renderablenodeline.cpp b/modules/base/rendering/renderablenodeline.cpp index 61d8d9c433..7cb476c62c 100644 --- a/modules/base/rendering/renderablenodeline.cpp +++ b/modules/base/rendering/renderablenodeline.cpp @@ -112,10 +112,10 @@ namespace { struct [[codegen::Dictionary(RenderableNodeLine)]] Parameters { // [[codegen::verbatim(StartNodeInfo.description)]] - std::optional startNode; + std::optional startNode [[codegen::identifier()]]; // [[codegen::verbatim(EndNodeInfo.description)]] - std::optional endNode; + std::optional endNode [[codegen::identifier()]]; // [[codegen::verbatim(LineColorInfo.description)]] std::optional color [[codegen::color()]]; diff --git a/modules/globebrowsing/src/layer.cpp b/modules/globebrowsing/src/layer.cpp index 14fe55707a..57ef993c45 100644 --- a/modules/globebrowsing/src/layer.cpp +++ b/modules/globebrowsing/src/layer.cpp @@ -96,8 +96,8 @@ namespace { }; struct [[codegen::Dictionary(Layer), codegen::noexhaustive()]] Parameters { - // The unique identifier for this layer. May not contain '.' or spaces - std::string identifier; + // The unique identifier for this layer. + std::string identifier [[codegen::identifier()]]; // A human-readable name for the user interface. If this is omitted, the // identifier is used instead diff --git a/modules/sync/syncs/httpsynchronization.cpp b/modules/sync/syncs/httpsynchronization.cpp index 49065125c1..6404c86f18 100644 --- a/modules/sync/syncs/httpsynchronization.cpp +++ b/modules/sync/syncs/httpsynchronization.cpp @@ -39,7 +39,7 @@ namespace { struct [[codegen::Dictionary(HttpSynchronization)]] Parameters { // The unique identifier for this resource that is used to request a set of files // from the synchronization servers - std::string identifier; + std::string identifier [[codegen::identifier()]]; // The version of this resource that should be requested int version; diff --git a/modules/sync/syncs/urlsynchronization.cpp b/modules/sync/syncs/urlsynchronization.cpp index d481d1ae2f..855b814504 100644 --- a/modules/sync/syncs/urlsynchronization.cpp +++ b/modules/sync/syncs/urlsynchronization.cpp @@ -43,7 +43,7 @@ namespace { // This identifier will be part of the used folder structure and, can be used to // manually find the downloaded folder in the synchronization folder - std::string identifier; + std::string identifier [[codegen::identifier()]]; // If this value is set to 'true' and it is not overwritten by the global // settings, the file(s) pointed to by this URLSynchronization will always be diff --git a/src/documentation/verifier.cpp b/src/documentation/verifier.cpp index dc52dcfbac..3501201735 100644 --- a/src/documentation/verifier.cpp +++ b/src/documentation/verifier.cpp @@ -231,6 +231,37 @@ std::string StringVerifier::type() const { return "String"; } +IdentifierVerifier::IdentifierVerifier() : StringVerifier(true) {} + +TestResult IdentifierVerifier::operator()(const ghoul::Dictionary& dict, + const std::string& key) const +{ + TestResult res = StringVerifier::operator()(dict, key); + if (!res.success) { + return res; + } + + std::string identifier = dict.value(key); + size_t pos = identifier.find_first_of(" \t\n\r."); + if (pos != std::string::npos) { + res.success = false; + TestResult::Offense off; + off.offender = key; + off.reason = TestResult::Offense::Reason::Verification; + off.explanation = "Identifier contained illegal character"; + res.offenses.push_back(off); + } + return res; +} + +std::string IdentifierVerifier::documentation() const { + return "An identifier string. May not contain '.', spaces, newlines, or tabs"; +}; + +std::string IdentifierVerifier::type() const { + return "Identifier"; +} + FileVerifier::FileVerifier() : StringVerifier(true) {} TestResult FileVerifier::operator()(const ghoul::Dictionary& dict, diff --git a/src/rendering/dashboarditem.cpp b/src/rendering/dashboarditem.cpp index d941888a9d..3eaf3df310 100644 --- a/src/rendering/dashboarditem.cpp +++ b/src/rendering/dashboarditem.cpp @@ -43,7 +43,7 @@ namespace { struct [[codegen::Dictionary(DashboardItem)]] Parameters { std::string type; - std::string identifier; + std::string identifier [[codegen::identifier()]]; std::optional guiName; diff --git a/src/rendering/screenspacerenderable.cpp b/src/rendering/screenspacerenderable.cpp index ec1c53ed18..eb613b5579 100644 --- a/src/rendering/screenspacerenderable.cpp +++ b/src/rendering/screenspacerenderable.cpp @@ -175,9 +175,8 @@ namespace { // This is the unique identifier for this screenspace renderable. It has to be // unique amongst all existing screenspace nodes that already have been added to - // the scene. The identifier is not allowed to have any whitespace or '.' and must - // not be empty - std::optional identifier; + // the scene. + std::optional identifier [[codegen::identifier()]]; // [[codegen::verbatim(EnabledInfo.description)]] std::optional enabled; diff --git a/src/scene/assetmanager.cpp b/src/scene/assetmanager.cpp index 85e56e4836..79006813d5 100644 --- a/src/scene/assetmanager.cpp +++ b/src/scene/assetmanager.cpp @@ -100,7 +100,7 @@ namespace { // A list of all identifiers that are exposed by this asset. This list is needed // to populate the descriptions in the main user interface - std::optional> identifiers; + std::optional> identifiers [[codegen::identifier()]]; }; #include "assetmanager_codegen.cpp" diff --git a/src/scene/lightsource.cpp b/src/scene/lightsource.cpp index b20dd801c2..9bb931b5bd 100644 --- a/src/scene/lightsource.cpp +++ b/src/scene/lightsource.cpp @@ -48,7 +48,7 @@ namespace { std::string type [[codegen::annotation("Must name a valid LightSource type")]]; // The identifier of the light source - std::string identifier; + std::string identifier [[codegen::identifier()]]; // [[codegen::verbatim(EnabledInfo.description)]] std::optional enabled; diff --git a/src/scene/scenegraphnode.cpp b/src/scene/scenegraphnode.cpp index b1596f3e6a..44b63c2de1 100644 --- a/src/scene/scenegraphnode.cpp +++ b/src/scene/scenegraphnode.cpp @@ -179,16 +179,13 @@ namespace { // The identifier of this scene graph node. This name must be unique among all // scene graph nodes that are loaded in a specific scene. If a duplicate is // detected the loading of the node will fail, as will all childing that depend on - // the node. The identifier must not contain any whitespaces or '.' - std::string identifier; + // the node. + std::string identifier [[codegen::identifier()]]; // This names the parent of the currently specified scene graph node. The parent // must already exist in the scene graph. If not specified, the node will be // attached to the root of the scene graph - std::optional parent - [[codegen::annotation( - "If specified, this must be a name for another scene graph node" - )]]; + std::optional parent [[codegen::identifier()]]; // The renderable that is to be created for this scene graph node. A renderable is // a component of a scene graph node that will lead to some visual result on the diff --git a/src/util/resourcesynchronization.cpp b/src/util/resourcesynchronization.cpp index cf0d841c07..1a7165bb7f 100644 --- a/src/util/resourcesynchronization.cpp +++ b/src/util/resourcesynchronization.cpp @@ -41,7 +41,7 @@ namespace { // A unique identifier that is used to reference this specific // ResourceSynchronization object - std::string identifier; + std::string identifier [[codegen::identifier()]]; // A user readable name of this synchronization std::string name; diff --git a/support/coding/codegen b/support/coding/codegen index e72b9d7852..72abde18b4 160000 --- a/support/coding/codegen +++ b/support/coding/codegen @@ -1 +1 @@ -Subproject commit e72b9d7852b57ce841dbf50a79360dadde290648 +Subproject commit 72abde18b40cb8490fc173bb2e8043fd43a8b48b diff --git a/tests/test_documentation.cpp b/tests/test_documentation.cpp index 66702f9789..4aa618087b 100644 --- a/tests/test_documentation.cpp +++ b/tests/test_documentation.cpp @@ -57,6 +57,11 @@ TEST_CASE("Documentation: Constructor", "[documentation]") { new StringVerifier, Optional::No ); + doc.entries.emplace_back( + "IdentifierVerifier", + new IdentifierVerifier, + Optional::No + ); doc.entries.emplace_back( "FileVerifier", new FileVerifier, @@ -279,6 +284,7 @@ TEST_CASE("Documentation: Initializer Constructor", "[documentation]") { {"DoubleVerifier", new DoubleVerifier, Optional::No }, {"IntVerifier", new IntVerifier, Optional::No }, {"StringVerifier", new StringVerifier, Optional::No }, + {"IdentifierVerifier", new IdentifierVerifier, Optional::No }, {"FileVerifier", new FileVerifier, Optional::No }, {"DirectoryVerifier", new DirectoryVerifier, Optional::No }, {"DateTimeVerifier", new DateTimeVerifier, Optional::No }, @@ -465,6 +471,69 @@ TEST_CASE("Documentation: StringVerifier", "[documentation]") { CHECK(negativeRes.offenses[0].reason == TestResult::Offense::Reason::MissingKey); } +TEST_CASE("Documentation: IdentifierVerifier", "[documentation]") { + using namespace openspace::documentation; + using namespace std::string_literals; + + Documentation doc{ + {{ "Identifier", new IdentifierVerifier, Optional::No }} + }; + + ghoul::Dictionary positive; + positive.setValue("Identifier", "abcdef"s); + TestResult positiveRes = testSpecification(doc, positive); + CHECK(positiveRes.success); + CHECK(positiveRes.offenses.empty()); + + ghoul::Dictionary negativeSpace; + negativeSpace.setValue("Identifier", "abc def"s); + TestResult negativeRes = testSpecification(doc, negativeSpace); + CHECK(!negativeRes.success); + REQUIRE(negativeRes.offenses.size() == 1); + CHECK(negativeRes.offenses[0].offender == "Identifier"); + CHECK(negativeRes.offenses[0].reason == TestResult::Offense::Reason::Verification); + + ghoul::Dictionary negativeTab; + negativeTab.setValue("Identifier", "abc\tdef"s); + negativeRes = testSpecification(doc, negativeTab); + CHECK(!negativeRes.success); + REQUIRE(negativeRes.offenses.size() == 1); + CHECK(negativeRes.offenses[0].offender == "Identifier"); + CHECK(negativeRes.offenses[0].reason == TestResult::Offense::Reason::Verification); + + ghoul::Dictionary negativeNewline; + negativeNewline.setValue("Identifier", "abc\ndef"s); + negativeRes = testSpecification(doc, negativeNewline); + CHECK(!negativeRes.success); + REQUIRE(negativeRes.offenses.size() == 1); + CHECK(negativeRes.offenses[0].offender == "Identifier"); + CHECK(negativeRes.offenses[0].reason == TestResult::Offense::Reason::Verification); + + ghoul::Dictionary negativeCarriageReturn; + negativeCarriageReturn.setValue("Identifier", "abc\rdef"s); + negativeRes = testSpecification(doc, negativeCarriageReturn); + CHECK(!negativeRes.success); + REQUIRE(negativeRes.offenses.size() == 1); + CHECK(negativeRes.offenses[0].offender == "Identifier"); + CHECK(negativeRes.offenses[0].reason == TestResult::Offense::Reason::Verification); + + ghoul::Dictionary negativeDot; + negativeDot.setValue("Identifier", "abc.def"s); + negativeRes = testSpecification(doc, negativeDot); + CHECK(!negativeRes.success); + REQUIRE(negativeRes.offenses.size() == 1); + CHECK(negativeRes.offenses[0].offender == "Identifier"); + CHECK(negativeRes.offenses[0].reason == TestResult::Offense::Reason::Verification); + + ghoul::Dictionary negativeType; + negativeType.setValue("Identifier", 0); + negativeRes = testSpecification(doc, negativeType); + CHECK_FALSE(negativeRes.success); + REQUIRE(negativeRes.offenses.size() == 1); + CHECK(negativeRes.offenses[0].offender == "Identifier"); + CHECK(negativeRes.offenses[0].reason == TestResult::Offense::Reason::WrongType); +} + TEST_CASE("Documentation: FileVerifier", "[documentation]") { using namespace openspace::documentation; using namespace std::string_literals;