Skip to content

Commit aae9d34

Browse files
MarenzMarenz
committed
Split DocStringParsing in two stages
one requiring type info in the next step
1 parent 1f8f1a3 commit aae9d34

4 files changed

Lines changed: 77 additions & 46 deletions

File tree

‎libsolidity/analysis/DocStringTagParser.cpp‎

Lines changed: 61 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,66 @@ bool DocStringTagParser::parseDocStrings(SourceUnit const& _sourceUnit)
4747
return errorWatcher.ok();
4848
}
4949

50+
bool DocStringTagParser::validateDocStringsUsingTypes(SourceUnit const& _sourceUnit)
51+
{
52+
auto errorWatcher = m_errorReporter.errorWatcher();
53+
54+
SimpleASTVisitor visitReturns(
55+
[](ASTNode const&) { return true; },
56+
[&](ASTNode const& _node)
57+
{
58+
if (auto const* annotation = dynamic_cast<StructurallyDocumentedAnnotation const*>(&_node.annotation()))
59+
{
60+
auto const& documentationNode = dynamic_cast<StructurallyDocumented const&>(_node);
61+
62+
size_t returnTagsVisited = 0;
63+
64+
for (auto const& [tagName, tagValue]: annotation->docTags)
65+
if (tagName == "return")
66+
{
67+
returnTagsVisited++;
68+
if (auto const* varDecl = dynamic_cast<VariableDeclaration const*>(&_node))
69+
{
70+
solAssert(varDecl->isPublic(), "@return is only allowed on public state-variables.");
71+
if (returnTagsVisited > 1)
72+
m_errorReporter.docstringParsingError(
73+
5256_error,
74+
documentationNode.documentation()->location(),
75+
"Documentation tag \"@" + tagName + "\" is only allowed once on state-variables."
76+
);
77+
}
78+
else if (auto const* function = dynamic_cast<FunctionDefinition const*>(&_node))
79+
{
80+
string content = tagValue.content;
81+
string firstWord = content.substr(0, content.find_first_of(" \t"));
82+
83+
if (returnTagsVisited > function->returnParameters().size())
84+
m_errorReporter.docstringParsingError(
85+
2604_error,
86+
documentationNode.documentation()->location(),
87+
"Documentation tag \"@" + tagName + " " + tagValue.content + "\"" +
88+
" exceeds the number of return parameters."
89+
);
90+
else
91+
{
92+
auto parameter = function->returnParameters().at(returnTagsVisited - 1);
93+
if (!parameter->name().empty() && parameter->name() != firstWord)
94+
m_errorReporter.docstringParsingError(
95+
5856_error,
96+
documentationNode.documentation()->location(),
97+
"Documentation tag \"@" + tagName + " " + tagValue.content + "\"" +
98+
" does not contain the name of its return parameter."
99+
);
100+
}
101+
}
102+
}
103+
}
104+
});
105+
106+
_sourceUnit.accept(visitReturns);
107+
return errorWatcher.ok();
108+
}
109+
50110
bool DocStringTagParser::visit(ContractDefinition const& _contract)
51111
{
52112
static set<string> const validTags = set<string>{"author", "title", "dev", "notice"};
@@ -169,7 +229,6 @@ void DocStringTagParser::parseDocStrings(
169229

170230
_annotation.docTags = DocStringParser{*_node.documentation(), m_errorReporter}.parse();
171231

172-
size_t returnTagsVisited = 0;
173232
for (auto const& [tagName, tagValue]: _annotation.docTags)
174233
{
175234
string static const customPrefix("custom:");
@@ -196,43 +255,6 @@ void DocStringTagParser::parseDocStrings(
196255
_node.documentation()->location(),
197256
"Documentation tag @" + tagName + " not valid for " + _nodeName + "."
198257
);
199-
else if (tagName == "return")
200-
{
201-
returnTagsVisited++;
202-
if (auto const* varDecl = dynamic_cast<VariableDeclaration const*>(&_node))
203-
{
204-
solAssert(varDecl->isPublic(), "@return is only allowed on public state-variables.");
205-
if (returnTagsVisited > 1)
206-
m_errorReporter.docstringParsingError(
207-
5256_error,
208-
_node.documentation()->location(),
209-
"Documentation tag \"@" + tagName + "\" is only allowed once on state-variables."
210-
);
211-
}
212-
else if (auto const* function = dynamic_cast<FunctionDefinition const*>(&_node))
213-
{
214-
string content = tagValue.content;
215-
string firstWord = content.substr(0, content.find_first_of(" \t"));
216-
217-
if (returnTagsVisited > function->returnParameters().size())
218-
m_errorReporter.docstringParsingError(
219-
2604_error,
220-
_node.documentation()->location(),
221-
"Documentation tag \"@" + tagName + " " + tagValue.content + "\"" +
222-
" exceeds the number of return parameters."
223-
);
224-
else
225-
{
226-
auto parameter = function->returnParameters().at(returnTagsVisited - 1);
227-
if (!parameter->name().empty() && parameter->name() != firstWord)
228-
m_errorReporter.docstringParsingError(
229-
5856_error,
230-
_node.documentation()->location(),
231-
"Documentation tag \"@" + tagName + " " + tagValue.content + "\"" +
232-
" does not contain the name of its return parameter."
233-
);
234-
}
235-
}
236-
}
237258
}
238259
}
260+

‎libsolidity/analysis/DocStringTagParser.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ class DocStringTagParser: private ASTConstVisitor
3737
public:
3838
explicit DocStringTagParser(langutil::ErrorReporter& _errorReporter): m_errorReporter(_errorReporter) {}
3939
bool parseDocStrings(SourceUnit const& _sourceUnit);
40+
/// Validate the parsed doc strings, requires parseDocStrings() and the
41+
/// DeclarationTypeChecker to have run.
42+
bool validateDocStringsUsingTypes(SourceUnit const& _sourceUnit);
4043

4144
private:
4245
bool visit(ContractDefinition const& _contract) override;

‎libsolidity/interface/CompilerStack.cpp‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -416,11 +416,6 @@ bool CompilerStack::analyze()
416416
if (source->ast && !syntaxChecker.checkSyntax(*source->ast))
417417
noErrors = false;
418418

419-
DocStringTagParser docStringTagParser(m_errorReporter);
420-
for (Source const* source: m_sourceOrder)
421-
if (source->ast && !docStringTagParser.parseDocStrings(*source->ast))
422-
noErrors = false;
423-
424419
m_globalContext = make_shared<GlobalContext>();
425420
// We need to keep the same resolver during the whole process.
426421
NameAndTypeResolver resolver(*m_globalContext, m_evmVersion, m_errorReporter);
@@ -437,6 +432,12 @@ bool CompilerStack::analyze()
437432

438433
resolver.warnHomonymDeclarations();
439434

435+
DocStringTagParser docStringTagParser(m_errorReporter);
436+
for (Source const* source: m_sourceOrder)
437+
if (source->ast && !docStringTagParser.parseDocStrings(*source->ast))
438+
noErrors = false;
439+
440+
// Requires DocStringTagParser
440441
for (Source const* source: m_sourceOrder)
441442
if (source->ast && !resolver.resolveNamesAndTypes(*source->ast))
442443
return false;
@@ -446,6 +447,11 @@ bool CompilerStack::analyze()
446447
if (source->ast && !declarationTypeChecker.check(*source->ast))
447448
return false;
448449

450+
// Requires DeclarationTypeChecker to have run
451+
for (Source const* source: m_sourceOrder)
452+
if (source->ast && !docStringTagParser.validateDocStringsUsingTypes(*source->ast))
453+
noErrors = false;
454+
449455
// Next, we check inheritance, overrides, function collisions and other things at
450456
// contract or function level.
451457
// This also calculates whether a contract is abstract, which is needed by the

‎test/libsolidity/SolidityNatspecJSON.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1274,7 +1274,7 @@ BOOST_AUTO_TEST_CASE(dev_default_inherit_variable)
12741274

12751275
char const *natspec1 = R"ABCDEF({
12761276
"methods" : {},
1277-
"stateVariables" :
1277+
"stateVariables" :
12781278
{
12791279
"x" :
12801280
{
@@ -1340,7 +1340,7 @@ BOOST_AUTO_TEST_CASE(dev_explicit_inherit_variable)
13401340

13411341
char const *natspec1 = R"ABCDEF({
13421342
"methods" : {},
1343-
"stateVariables" :
1343+
"stateVariables" :
13441344
{
13451345
"x" :
13461346
{

0 commit comments

Comments
 (0)