Skip to content

Commit

Permalink
Report Unknown Doc-Comment Tags in Slice (#3127)
Browse files Browse the repository at this point in the history
  • Loading branch information
@InsertCreativityHere
InsertCreativityHere authored Nov 13, 2024
1 parent 53f04f9 commit 6e7c14b
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 138 deletions.
81 changes: 45 additions & 36 deletions cpp/src/Slice/Parser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,10 @@ Slice::DefinitionContext::initSuppressedWarnings()
{
_suppressedWarnings.insert(InvalidMetadata);
}
else if (s == "invalid-comment")
{
_suppressedWarnings.insert(InvalidComment);
}
else
{
ostringstream os;
Expand Down Expand Up @@ -320,12 +324,6 @@ Slice::Comment::overview() const
return _overview;
}

StringList
Slice::Comment::misc() const
{
return _misc;
}

StringList
Slice::Comment::seeAlso() const
{
Expand Down Expand Up @@ -777,95 +775,99 @@ Slice::Contained::parseComment(function<string(string, string)> 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:
Expand All @@ -892,6 +894,14 @@ Slice::Contained::parseComment(function<string(string, string)> 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);
Expand All @@ -908,7 +918,6 @@ Slice::Contained::parseComment(function<string(string, string)> linkFormatter, b

trimLines(comment->_overview);
trimLines(comment->_deprecated);
trimLines(comment->_misc);
trimLines(comment->_returns);

return comment;
Expand Down
8 changes: 3 additions & 5 deletions cpp/src/Slice/Parser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ namespace Slice
{
All,
Deprecated,
InvalidMetadata
InvalidMetadata,
InvalidComment
};

class GrammarBase;
Expand Down Expand Up @@ -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.
Expand All @@ -270,7 +269,6 @@ namespace Slice
bool _isDeprecated;
StringList _deprecated;
StringList _overview;
StringList _misc;
StringList _seeAlso;

StringList _returns;
Expand Down
11 changes: 0 additions & 11 deletions cpp/src/slice2cpp/Gen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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())
{
Expand Down
18 changes: 0 additions & 18 deletions cpp/src/slice2java/Gen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 << " *";
Expand Down Expand Up @@ -2007,12 +2001,6 @@ Slice::JavaVisitor::writeProxyDocComment(
}
}

if (!dc->misc().empty())
{
out << nl << " * ";
writeDocCommentLines(out, dc->misc());
}

if (!dc->seeAlso().empty())
{
out << nl << " *";
Expand Down Expand Up @@ -2172,12 +2160,6 @@ Slice::JavaVisitor::writeServantDocComment(
}
}

if (!dc->misc().empty())
{
out << nl << " * ";
writeDocCommentLines(out, dc->misc());
}

if (!dc->seeAlso().empty())
{
out << nl << " *";
Expand Down
10 changes: 0 additions & 10 deletions cpp/src/slice2js/Gen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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());
Expand Down
28 changes: 0 additions & 28 deletions cpp/src/slice2matlab/Main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
{
Expand Down Expand Up @@ -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())
{
Expand Down Expand Up @@ -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())
{
Expand Down Expand Up @@ -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();
Expand All @@ -1287,12 +1265,6 @@ namespace
writeDocLines(out, docOverview, false);
}

if (!docMisc.empty())
{
out << nl << "%";
writeDocLines(out, docMisc, true);
}

if (!docSeeAlso.empty())
{
out << nl << "%";
Expand Down
Loading

0 comments on commit 6e7c14b

Please sign in to comment.