From 6e7c14bfdb4cadb1b3aa10d1530d4770a62c0d9c Mon Sep 17 00:00:00 2001 From: Austin Henriksen Date: Wed, 13 Nov 2024 10:01:23 -0500 Subject: [PATCH] Report Unknown Doc-Comment Tags in Slice (#3127) --- cpp/src/Slice/Parser.cpp | 81 ++++++++++--------- cpp/src/Slice/Parser.h | 8 +- cpp/src/slice2cpp/Gen.cpp | 11 --- cpp/src/slice2java/Gen.cpp | 18 ----- cpp/src/slice2js/Gen.cpp | 10 --- cpp/src/slice2matlab/Main.cpp | 28 ------- cpp/src/slice2swift/SwiftUtil.cpp | 30 ------- .../errorDetection/WarningInvalidComment.err | 3 + .../errorDetection/WarningInvalidComment.ice | 11 +++ 9 files changed, 62 insertions(+), 138 deletions(-) create mode 100644 cpp/test/Slice/errorDetection/WarningInvalidComment.err create mode 100644 cpp/test/Slice/errorDetection/WarningInvalidComment.ice diff --git a/cpp/src/Slice/Parser.cpp b/cpp/src/Slice/Parser.cpp index 1c48f5f272b..e48dbf40750 100644 --- a/cpp/src/Slice/Parser.cpp +++ b/cpp/src/Slice/Parser.cpp @@ -287,6 +287,10 @@ Slice::DefinitionContext::initSuppressedWarnings() { _suppressedWarnings.insert(InvalidMetadata); } + else if (s == "invalid-comment") + { + _suppressedWarnings.insert(InvalidComment); + } else { ostringstream os; @@ -320,12 +324,6 @@ Slice::Comment::overview() const return _overview; } -StringList -Slice::Comment::misc() const -{ - return _misc; -} - StringList Slice::Comment::seeAlso() const { @@ -777,95 +775,99 @@ Slice::Contained::parseComment(function linkFormatter, b enum State { - StateMisc, + StateUnknown, StateParam, StateThrows, + StateSee, StateReturn, StateDeprecated }; - State state = StateMisc; + State state = StateUnknown; string name; const string ws = " \t"; const string paramTag = "@param"; const string throwsTag = "@throws"; const string exceptionTag = "@exception"; + const string seeTag = "@see"; const string returnTag = "@return"; const string deprecatedTag = "@deprecated"; - const string seeTag = "@see"; for (; i != lines.end(); ++i) { const string l = IceInternal::trim(*i); - string line; - if (parseCommentLine(l, paramTag, true, name, line)) + string lineText; + if (parseCommentLine(l, paramTag, true, name, lineText)) { - if (!line.empty()) + if (!lineText.empty()) { state = StateParam; StringList sl; - sl.push_back(line); // The first line of the description. + sl.push_back(lineText); // The first line of the description. comment->_parameters[name] = sl; } } - else if (parseCommentLine(l, throwsTag, true, name, line)) + else if (parseCommentLine(l, throwsTag, true, name, lineText)) { - if (!line.empty()) + if (!lineText.empty()) { state = StateThrows; StringList sl; - sl.push_back(line); // The first line of the description. + sl.push_back(lineText); // The first line of the description. comment->_exceptions[name] = sl; } } - else if (parseCommentLine(l, exceptionTag, true, name, line)) + else if (parseCommentLine(l, exceptionTag, true, name, lineText)) { - if (!line.empty()) + if (!lineText.empty()) { state = StateThrows; StringList sl; - sl.push_back(line); // The first line of the description. + sl.push_back(lineText); // The first line of the description. comment->_exceptions[name] = sl; } } - else if (parseCommentLine(l, seeTag, false, name, line)) + else if (parseCommentLine(l, seeTag, false, name, lineText)) { - if (!line.empty()) + if (!lineText.empty()) { - state = StateMisc; // Reset the state; `@see` cannot span multiple lines. - comment->_seeAlso.push_back(line); + state = StateSee; + comment->_seeAlso.push_back(lineText); } } - else if (parseCommentLine(l, returnTag, false, name, line)) + else if (parseCommentLine(l, returnTag, false, name, lineText)) { - if (!line.empty()) + if (!lineText.empty()) { state = StateReturn; - comment->_returns.push_back(line); // The first line of the description. + comment->_returns.push_back(lineText); // The first line of the description. } } - else if (parseCommentLine(l, deprecatedTag, false, name, line)) + else if (parseCommentLine(l, deprecatedTag, false, name, lineText)) { comment->_isDeprecated = true; - if (!line.empty()) + if (!lineText.empty()) { state = StateDeprecated; - comment->_deprecated.push_back(line); // The first line of the description. + comment->_deprecated.push_back(lineText); // The first line of the description. } } else if (!l.empty()) { if (l[0] == '@') { - // - // Treat all other tags as miscellaneous comments. - // - state = StateMisc; + // We've encountered an unknown doc tag. + auto unknownTag = l.substr(0, l.find_first_of(" \t:")); + string msg = "ignoring unknown doc tag '" + unknownTag + "' in comment"; + unit()->warning(file(), line(), InvalidComment, msg); + state = StateUnknown; + continue; } switch (state) { - case StateMisc: + case StateUnknown: { - comment->_misc.push_back(l); + // Getting here means we've hit an unknown tag that spanned multiple lines. + // We immediately break - ignoring and discarding the line. break; } case StateParam: @@ -892,6 +894,14 @@ Slice::Contained::parseComment(function linkFormatter, b comment->_exceptions[name] = sl; break; } + case StateSee: + { + // This isn't allowed - '@see' tags cannot span multiple lines. We've already kept the original + // line by this point, but we ignore any lines that follow it (until hitting another '@' tag). + string msg = "'@see' tags cannot span multiple lines and must be of the form: '@see identifier'"; + unit()->warning(file(), line(), InvalidComment, msg); + break; + } case StateReturn: { comment->_returns.push_back(l); @@ -908,7 +918,6 @@ Slice::Contained::parseComment(function linkFormatter, b trimLines(comment->_overview); trimLines(comment->_deprecated); - trimLines(comment->_misc); trimLines(comment->_returns); return comment; diff --git a/cpp/src/Slice/Parser.h b/cpp/src/Slice/Parser.h index 1e5d97837e0..a7453636c36 100644 --- a/cpp/src/Slice/Parser.h +++ b/cpp/src/Slice/Parser.h @@ -38,7 +38,8 @@ namespace Slice { All, Deprecated, - InvalidMetadata + InvalidMetadata, + InvalidComment }; class GrammarBase; @@ -252,9 +253,7 @@ namespace Slice /// Contains all introductory lines up to the first tag. StringList overview() const; - /// Contains unrecognized tags. - StringList misc() const; - /// Targets of @see tags. + /// Targets of '@see' tags. StringList seeAlso() const; /// Description of an operation's return value. @@ -270,7 +269,6 @@ namespace Slice bool _isDeprecated; StringList _deprecated; StringList _overview; - StringList _misc; StringList _seeAlso; StringList _returns; diff --git a/cpp/src/slice2cpp/Gen.cpp b/cpp/src/slice2cpp/Gen.cpp index 9c8cee9790f..cfb2768917f 100644 --- a/cpp/src/slice2cpp/Gen.cpp +++ b/cpp/src/slice2cpp/Gen.cpp @@ -398,11 +398,6 @@ namespace writeDocLines(out, doc->overview(), true); } - if (!doc->misc().empty()) - { - writeDocLines(out, doc->misc(), true); - } - if (!doc->seeAlso().empty()) { writeSeeAlso(out, doc->seeAlso()); @@ -541,12 +536,6 @@ namespace writeOpDocExceptions(out, op, doc); } - const auto& misc = doc->misc(); - if (!misc.empty()) - { - writeDocLines(out, misc, true); - } - const auto& seeAlso = doc->seeAlso(); if (!seeAlso.empty()) { diff --git a/cpp/src/slice2java/Gen.cpp b/cpp/src/slice2java/Gen.cpp index 1ba86b00886..7b267ef00b6 100644 --- a/cpp/src/slice2java/Gen.cpp +++ b/cpp/src/slice2java/Gen.cpp @@ -1860,12 +1860,6 @@ Slice::JavaVisitor::writeDocComment(Output& out, const UnitPtr& unt, const Comme writeDocCommentLines(out, dc->overview()); } - if (!dc->misc().empty()) - { - out << nl << " * "; - writeDocCommentLines(out, dc->misc()); - } - if (!dc->seeAlso().empty()) { out << nl << " *"; @@ -2007,12 +2001,6 @@ Slice::JavaVisitor::writeProxyDocComment( } } - if (!dc->misc().empty()) - { - out << nl << " * "; - writeDocCommentLines(out, dc->misc()); - } - if (!dc->seeAlso().empty()) { out << nl << " *"; @@ -2172,12 +2160,6 @@ Slice::JavaVisitor::writeServantDocComment( } } - if (!dc->misc().empty()) - { - out << nl << " * "; - writeDocCommentLines(out, dc->misc()); - } - if (!dc->seeAlso().empty()) { out << nl << " *"; diff --git a/cpp/src/slice2js/Gen.cpp b/cpp/src/slice2js/Gen.cpp index 353490d8708..55bfc689d55 100644 --- a/cpp/src/slice2js/Gen.cpp +++ b/cpp/src/slice2js/Gen.cpp @@ -495,11 +495,6 @@ Slice::JsVisitor::writeDocCommentFor(const ContainedPtr& p, bool includeDeprecat writeDocLines(_out, comment->overview(), true); } - if (!comment->misc().empty()) - { - writeDocLines(_out, comment->misc(), true); - } - if (!comment->seeAlso().empty()) { writeSeeAlso(_out, comment->seeAlso()); @@ -2504,11 +2499,6 @@ Slice::Gen::TypeScriptVisitor::writeOpDocSummary( { writeOpDocExceptions(out, op, comment); - if (!comment->misc().empty()) - { - writeDocLines(out, comment->misc(), true); - } - if (!comment->seeAlso().empty()) { writeSeeAlso(out, comment->seeAlso()); diff --git a/cpp/src/slice2matlab/Main.cpp b/cpp/src/slice2matlab/Main.cpp index 17d1dc80767..4c2c80127c1 100644 --- a/cpp/src/slice2matlab/Main.cpp +++ b/cpp/src/slice2matlab/Main.cpp @@ -970,13 +970,6 @@ namespace } } - StringList docMisc = doc->misc(); - if (!docMisc.empty()) - { - out << nl << "%"; - writeDocLines(out, docMisc, true); - } - StringList docSeeAlso = doc->seeAlso(); if (!docSeeAlso.empty()) { @@ -1138,13 +1131,6 @@ namespace } } - StringList docMisc = doc->misc(); - if (!docMisc.empty()) - { - out << nl << "%"; - writeDocLines(out, docMisc, true); - } - StringList docSeeAlso = doc->seeAlso(); if (!docSeeAlso.empty()) { @@ -1224,13 +1210,6 @@ namespace out << nl << "% checkedCast - Contacts the remote server to verify that the object implements this type."; out << nl << "% uncheckedCast - Downcasts the given proxy to this type without contacting the remote server."; - StringList docMisc = doc->misc(); - if (!docMisc.empty()) - { - out << nl << "%"; - writeDocLines(out, docMisc, true); - } - StringList docSeeAlso = doc->seeAlso(); if (!docSeeAlso.empty()) { @@ -1263,7 +1242,6 @@ namespace } StringList docOverview = doc->overview(); - StringList docMisc = doc->misc(); StringList docSeeAlso = doc->seeAlso(); StringList docDeprecated = doc->deprecated(); bool docIsDeprecated = doc->isDeprecated(); @@ -1287,12 +1265,6 @@ namespace writeDocLines(out, docOverview, false); } - if (!docMisc.empty()) - { - out << nl << "%"; - writeDocLines(out, docMisc, true); - } - if (!docSeeAlso.empty()) { out << nl << "%"; diff --git a/cpp/src/slice2swift/SwiftUtil.cpp b/cpp/src/slice2swift/SwiftUtil.cpp index 8c29ada16b5..eafd7771195 100644 --- a/cpp/src/slice2swift/SwiftUtil.cpp +++ b/cpp/src/slice2swift/SwiftUtil.cpp @@ -361,17 +361,6 @@ SwiftGenerator::writeDocSummary(IceInternal::Output& out, const ContainedPtr& p) hasStarted = true; } - StringList docMisc = doc->misc(); - if (!docMisc.empty()) - { - if (hasStarted) - { - out << nl << "///"; - } - hasStarted = true; - writeDocLines(out, docMisc); - } - if (doc->isDeprecated()) { if (hasStarted) @@ -532,13 +521,6 @@ SwiftGenerator::writeOpDocSummary(IceInternal::Output& out, const OperationPtr& } } } - - StringList docMisc = doc->misc(); - if (!docMisc.empty()) - { - out << nl; - writeDocLines(out, docMisc, false); - } } void @@ -596,12 +578,6 @@ SwiftGenerator::writeProxyDocSummary(IceInternal::Output& out, const InterfaceDe } } } - - StringList docMisc = doc->misc(); - if (!docMisc.empty()) - { - writeDocLines(out, docMisc, false); - } } void @@ -645,12 +621,6 @@ SwiftGenerator::writeServantDocSummary(IceInternal::Output& out, const Interface } } } - - StringList docMisc = doc->misc(); - if (!docMisc.empty()) - { - writeDocLines(out, docMisc, false); - } } void diff --git a/cpp/test/Slice/errorDetection/WarningInvalidComment.err b/cpp/test/Slice/errorDetection/WarningInvalidComment.err new file mode 100644 index 00000000000..9e20b223c34 --- /dev/null +++ b/cpp/test/Slice/errorDetection/WarningInvalidComment.err @@ -0,0 +1,3 @@ +WarningInvalidComment.ice:10: warning: ignoring unknown doc tag '@remarks' in comment +WarningInvalidComment.ice:10: warning: '@see' tags cannot span multiple lines and must be of the form: '@see identifier' +WarningInvalidComment.ice:10: warning: ignoring unknown doc tag '@bad' in comment diff --git a/cpp/test/Slice/errorDetection/WarningInvalidComment.ice b/cpp/test/Slice/errorDetection/WarningInvalidComment.ice new file mode 100644 index 00000000000..a8ba2f37917 --- /dev/null +++ b/cpp/test/Slice/errorDetection/WarningInvalidComment.ice @@ -0,0 +1,11 @@ + +module Test +{ + /// This is a test overview. + /// @remarks: This is an unknown comment tag which spans 1 line. + /// @see: CommentDummy + /// But then we write a 2nd line, which isn't allowed for 'see' tags. + /// @bad: Another unknown comment tag, but this will span 2 lines. + /// This 2nd line should be ignored, but won't trigger another error. + class CommentDummy {} +}