Finish Operation Doc Comment Validation #3852
Conversation
| else if ( | ||
| parseNamedCommentLine(l, throwsTag, name, lineText) || | ||
| parseNamedCommentLine(l, exceptionTag, name, lineText)) |
There was a problem hiding this comment.
We support @exception as an alias for @throws, and used to parse/validate them separately.
I combined them both into a single block to avoid code-duplication.
| // Check that the '@param <name>' corresponds to an actual parameter in the operation. | ||
| const ParameterList params = operationTarget->parameters(); | ||
| const auto paramNameCheck = [&name](const ParameterPtr& param) { return param->name() == name; }; | ||
| if (std::find_if(params.begin(), params.end(), paramNameCheck) == params.end()) |
There was a problem hiding this comment.
You should use std::none_of. I would also put the lambda inline, no need to give it a name.
https://en.cppreference.com/w/cpp/algorithm/all_any_none_of.
| @@ -522,47 +522,88 @@ Slice::DocComment::parseFrom(const ContainedPtr& p, DocLinkFormatter linkFormatt | |||
|
|
|||
| if (parseNamedCommentLine(l, paramTag, name, lineText)) | |||
There was a problem hiding this comment.
It would be really nice to avoid naming anying l!
| if (std::find_if(exceptionSpec.begin(), exceptionSpec.end(), exceptionCheck) == exceptionSpec.end()) | ||
| { | ||
| const string msg = "'" + actualTag + " " + name + "': this exception is not listed in " + | ||
| "(or a sub-exception of) this operation's exception specification"; |
There was a problem hiding this comment.
The sentence is not grammatically correct.
| // Check if this is a duplicate tag. If it is, ignore it and issue a warning. | ||
| if (comment._parameters.count(name) != 0) | ||
| { | ||
| const string msg = "ignoring duplicate doc-comment tag: '" + paramTag + " " + name + "'"; |
There was a problem hiding this comment.
| const string msg = "ignoring duplicate doc-comment tag: '" + paramTag + " " + name + "'"; | |
| const string msg = "duplicate doc-comment tag: '" + paramTag + " " + name + "'"; |
| const string msg = "'" + actualTag + " " + name + "': this exception is not listed in " + | ||
| "(or a sub-exception of) this operation's exception specification"; |
There was a problem hiding this comment.
Maybe something like:
const string msg = "'" + actualTag + " " + name +
"': this exception is neither listed in nor derived from any exception in the operation's exception specification";
This PR finishes implementing #2998.
Now the parser will:
@param fooiffooisn't the name of an operation parameter@returnif it's applied to avoidoperation@throws FooifFooisn't in it's exception specific (or derived from an exception that is)We also now reject duplicate tags.
So a comment can't have more than one
@deprecated, or more than one@return.And for 'named' tags, you can't use the same name more than once either.