From 15333abed9112f99e0b1af4327154af733b987d3 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 16 Feb 2024 15:58:29 +0100 Subject: qapi: Improve error position for bogus argument descriptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When documented arguments don't exist, the error message points to the beginning of the definition comment. Point to the first bogus argument description instead. Signed-off-by: Markus Armbruster Message-ID: <20240216145841.2099240-6-armbru@redhat.com> Reviewed-by: Daniel P. Berrangé --- scripts/qapi/parser.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 88221b3..82db595 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -472,6 +472,8 @@ class QAPIDoc: # pylint: disable=too-few-public-methods def __init__(self, parser: QAPISchemaParser, name: Optional[str] = None): + # section source info, i.e. where it begins + self.info = parser.info # parser, for error messages about indentation self._parser = parser # optional section name (argument/member or section name) @@ -770,7 +772,7 @@ class QAPIDoc: if not section.member] if bogus: raise QAPISemError( - self.info, + args[bogus[0]].info, "documented %s%s '%s' %s not exist" % ( what, "s" if len(bogus) > 1 else "", -- cgit v1.1 From bf00dc19f3aacf014b308d57bb0debf250339396 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 16 Feb 2024 15:58:30 +0100 Subject: qapi: Improve error position for bogus invalid "Returns" section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When something other than a command has a "Returns" section, the error message points to the beginning of the definition comment. Point to the "Returns" section instead. Signed-off-by: Markus Armbruster Message-ID: <20240216145841.2099240-7-armbru@redhat.com> Reviewed-by: Daniel P. Berrangé --- scripts/qapi/parser.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 82db595..a771013 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -759,9 +759,13 @@ class QAPIDoc: self.features[feature.name].connect(feature) def check_expr(self, expr: QAPIExpression) -> None: - if self.has_section('Returns') and 'command' not in expr: - raise QAPISemError(self.info, - "'Returns:' is only valid for commands") + if 'command' not in expr: + sec = next((sec for sec in self.sections + if sec.name == 'Returns'), + None) + if sec: + raise QAPISemError(sec.info, + "'Returns:' is only valid for commands") def check(self) -> None: -- cgit v1.1 From 573e2223f91a1662dad3c4ab5f6724bbe2633eff Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 16 Feb 2024 15:58:31 +0100 Subject: qapi: Improve error message for empty doc sections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Improve the message for an empty tagged section from empty doc section 'Note' to text required after 'Note:' and the message for an empty argument or feature description from empty doc section 'foo' to text required after '@foo:' Improve the error position to refer to the beginning of the empty section instead of its end. Signed-off-by: Markus Armbruster Message-ID: <20240216145841.2099240-8-armbru@redhat.com> Reviewed-by: Daniel P. Berrangé --- scripts/qapi/parser.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index a771013..43daf55 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -476,9 +476,9 @@ class QAPIDoc: self.info = parser.info # parser, for error messages about indentation self._parser = parser - # optional section name (argument/member or section name) + # section tag, if any ('Returns', '@name', ...) self.name = name - # section text without section name + # section text without tag self.text = '' # indentation to strip (None means indeterminate) self._indent = None if self.name else 0 @@ -700,7 +700,7 @@ class QAPIDoc: raise QAPIParseError(self._parser, "'%s' parameter name duplicated" % name) assert not self.sections - new_section = QAPIDoc.ArgSection(self._parser, name) + new_section = QAPIDoc.ArgSection(self._parser, '@' + name) self._switch_section(new_section) symbols_dict[name] = new_section @@ -727,9 +727,9 @@ class QAPIDoc: # We do not create anonymous sections unless there is # something to put in them; this is a parser bug. assert self._section.name - raise QAPIParseError( - self._parser, - "empty doc section '%s'" % self._section.name) + raise QAPISemError( + self._section.info, + "text required after '%s:'" % self._section.name) self._section = new_section @@ -748,7 +748,7 @@ class QAPIDoc: "%s '%s' lacks documentation" % (member.role, member.name)) self.args[member.name] = QAPIDoc.ArgSection(self._parser, - member.name) + '@' + member.name) self.args[member.name].connect(member) def connect_feature(self, feature: 'QAPISchemaFeature') -> None: -- cgit v1.1 From 31c54b92ad0816ab1c4eddaf4b60c0b17a75dfc9 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 16 Feb 2024 15:58:32 +0100 Subject: qapi: Rename QAPIDoc.Section.name to .tag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since the previous commit, QAPIDoc.Section.name is either None (untagged section) or the section's tag string ('Returns', '@name', ...). Rename it to .tag. Signed-off-by: Markus Armbruster Message-ID: <20240216145841.2099240-9-armbru@redhat.com> Reviewed-by: Daniel P. Berrangé --- scripts/qapi/parser.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 43daf55..cc69f4f 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -471,17 +471,17 @@ class QAPIDoc: class Section: # pylint: disable=too-few-public-methods def __init__(self, parser: QAPISchemaParser, - name: Optional[str] = None): + tag: Optional[str] = None): # section source info, i.e. where it begins self.info = parser.info # parser, for error messages about indentation self._parser = parser # section tag, if any ('Returns', '@name', ...) - self.name = name + self.tag = tag # section text without tag self.text = '' # indentation to strip (None means indeterminate) - self._indent = None if self.name else 0 + self._indent = None if self.tag else 0 def append(self, line: str) -> None: line = line.rstrip() @@ -504,8 +504,8 @@ class QAPIDoc: class ArgSection(Section): def __init__(self, parser: QAPISchemaParser, - name: str): - super().__init__(parser, name) + tag: str): + super().__init__(parser, tag) self.member: Optional['QAPISchemaMember'] = None def connect(self, member: 'QAPISchemaMember') -> None: @@ -536,10 +536,10 @@ class QAPIDoc: self._section = self.body self._append_line = self._append_body_line - def has_section(self, name: str) -> bool: - """Return True if we have a section with this name.""" + def has_section(self, tag: str) -> bool: + """Return True if we have a section with this tag.""" for i in self.sections: - if i.name == name: + if i.tag == tag: return True return False @@ -710,11 +710,11 @@ class QAPIDoc: def _start_features_section(self, name: str) -> None: self._start_symbol_section(self.features, name) - def _start_section(self, name: Optional[str] = None) -> None: - if name in ('Returns', 'Since') and self.has_section(name): + def _start_section(self, tag: Optional[str] = None) -> None: + if tag in ('Returns', 'Since') and self.has_section(tag): raise QAPIParseError(self._parser, - "duplicated '%s' section" % name) - new_section = QAPIDoc.Section(self._parser, name) + "duplicated '%s' section" % tag) + new_section = QAPIDoc.Section(self._parser, tag) self._switch_section(new_section) self.sections.append(new_section) @@ -726,10 +726,10 @@ class QAPIDoc: if self._section != self.body and not text: # We do not create anonymous sections unless there is # something to put in them; this is a parser bug. - assert self._section.name + assert self._section.tag raise QAPISemError( self._section.info, - "text required after '%s:'" % self._section.name) + "text required after '%s:'" % self._section.tag) self._section = new_section @@ -761,7 +761,7 @@ class QAPIDoc: def check_expr(self, expr: QAPIExpression) -> None: if 'command' not in expr: sec = next((sec for sec in self.sections - if sec.name == 'Returns'), + if sec.tag == 'Returns'), None) if sec: raise QAPISemError(sec.info, -- cgit v1.1 From 56c64dd60aebb9c856ab63de74c9e81acd079436 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 16 Feb 2024 15:58:33 +0100 Subject: qapi: Reject section heading in the middle of a doc comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit docs/devel/qapi-code-gen.txt claims "A heading line must be the first line of the documentation comment block" since commit 55ec69f8b16 (docs/devel/qapi-code-gen.txt: Update to new rST backend conventions). Not true, we have code to make it work anywhere in a free-form doc comment: commit dcdc07a97cb (qapi: Make section headings start a new doc comment block). Make it true, for simplicity's sake. Signed-off-by: Markus Armbruster Message-ID: <20240216145841.2099240-10-armbru@redhat.com> Reviewed-by: Daniel P. Berrangé --- scripts/qapi/parser.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index cc69f4f..3aefec1 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -440,9 +440,9 @@ class QAPISchemaParser: self, "unexpected '=' markup in definition documentation") if cur_doc.body.text: - cur_doc.end_comment() - docs.append(cur_doc) - cur_doc = QAPIDoc(self, info) + raise QAPIParseError( + self, + "'=' heading must come first in a comment block") cur_doc.append(self.val) self.accept(False) -- cgit v1.1 From d23055b8db88a54b372ebbbffe3681169d2a767a Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 16 Feb 2024 15:58:34 +0100 Subject: qapi: Require descriptions and tagged sections to be indented MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By convention, we indent the second and subsequent lines of descriptions and tagged sections, except for examples. Turn this into a hard rule, and apply it to examples, too. Signed-off-by: Markus Armbruster Message-ID: <20240216145841.2099240-11-armbru@redhat.com> Reviewed-by: Daniel P. Berrangé [Straightforward conflicts in qapi/migration.json resolved] --- scripts/qapi/parser.py | 3 +++ 1 file changed, 3 insertions(+) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 3aefec1..f8da315 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -492,6 +492,9 @@ class QAPIDoc: # indeterminate indentation if self.text != '': # non-blank, non-first line determines indentation + if indent == 0: + raise QAPIParseError( + self._parser, "line needs to be indented") self._indent = indent elif indent < self._indent: raise QAPIParseError( -- cgit v1.1 From 66227e90478b34a2bc4bbd4f11f5ea637184c20b Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 16 Feb 2024 15:58:35 +0100 Subject: qapi: Recognize section tags and 'Features:' only after blank line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Putting a blank line before section tags and 'Features:' is good, existing practice. Enforce it. Signed-off-by: Markus Armbruster Message-ID: <20240216145841.2099240-12-armbru@redhat.com> Reviewed-by: Daniel P. Berrangé --- scripts/qapi/parser.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index f8da315..de2ce3e 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -538,6 +538,7 @@ class QAPIDoc: # the current section self._section = self.body self._append_line = self._append_body_line + self._first_line_in_paragraph = False def has_section(self, tag: str) -> bool: """Return True if we have a section with this tag.""" @@ -560,12 +561,14 @@ class QAPIDoc: line = line[1:] if not line: self._append_freeform(line) + self._first_line_in_paragraph = True return if line[0] != ' ': raise QAPIParseError(self._parser, "missing space after #") line = line[1:] self._append_line(line) + self._first_line_in_paragraph = False def end_comment(self) -> None: self._switch_section(QAPIDoc.NullSection(self._parser)) @@ -574,9 +577,11 @@ class QAPIDoc: def _match_at_name_colon(string: str) -> Optional[Match[str]]: return re.match(r'@([^:]*): *', string) - @staticmethod - def _match_section_tag(string: str) -> Optional[Match[str]]: - return re.match(r'(Returns|Since|Notes?|Examples?|TODO): *', string) + def _match_section_tag(self, string: str) -> Optional[Match[str]]: + if not self._first_line_in_paragraph: + return None + return re.match(r'(Returns|Since|Notes?|Examples?|TODO): *', + string) def _append_body_line(self, line: str) -> None: """ -- cgit v1.1 From 0b82a7440c22056745a925d0b1c070e18534aa0e Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 16 Feb 2024 15:58:37 +0100 Subject: qapi: Merge adjacent untagged sections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parser mostly doesn't create adjacent untagged sections, and merging the ones it does create is hardly worth the bother. I'm doing it to avoid behavioral change in the next commit. Signed-off-by: Markus Armbruster Message-ID: <20240216145841.2099240-14-armbru@redhat.com> Reviewed-by: Daniel P. Berrangé --- scripts/qapi/parser.py | 3 +++ 1 file changed, 3 insertions(+) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index de2ce3e..48cc9a6 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -719,6 +719,9 @@ class QAPIDoc: self._start_symbol_section(self.features, name) def _start_section(self, tag: Optional[str] = None) -> None: + if not tag and not self._section.tag: + # extend current section + return if tag in ('Returns', 'Since') and self.has_section(tag): raise QAPIParseError(self._parser, "duplicated '%s' section" % tag) -- cgit v1.1 From 3d035cd2cca66488f6f478a93b231c302466116b Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 16 Feb 2024 15:58:38 +0100 Subject: qapi: Rewrite doc comment parser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QAPISchemaParser is a conventional recursive descent parser. Except QAPISchemaParser.get_doc() delegates most of the doc comment parsing work to a state machine in QAPIDoc. The state machine doesn't get tokens like a recursive descent parser, it is fed tokens. I find this state machine rather opaque and hard to maintain. Replace it by a conventional parser, all in QAPISchemaParser. Less code, and (at least in my opinion) easier to understand. Signed-off-by: Markus Armbruster Message-ID: <20240216145841.2099240-15-armbru@redhat.com> Tested-by: Daniel P. Berrangé --- scripts/qapi/parser.py | 480 ++++++++++++++++++++++--------------------------- 1 file changed, 211 insertions(+), 269 deletions(-) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 48cc9a6..73ff150 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -134,8 +134,8 @@ class QAPISchemaParser: info = self.info if self.tok == '#': self.reject_expr_doc(cur_doc) - for cur_doc in self.get_doc(info): - self.docs.append(cur_doc) + cur_doc = self.get_doc() + self.docs.append(cur_doc) continue expr = self.get_expr() @@ -414,39 +414,171 @@ class QAPISchemaParser: self, "expected '{', '[', string, or boolean") return expr - def get_doc(self, info: QAPISourceInfo) -> List['QAPIDoc']: + def get_doc_line(self) -> Optional[str]: + if self.tok != '#': + raise QAPIParseError( + self, "documentation comment must end with '##'") + assert isinstance(self.val, str) + if self.val.startswith('##'): + # End of doc comment + if self.val != '##': + raise QAPIParseError( + self, "junk after '##' at end of documentation comment") + return None + if self.val == '#': + return '' + if self.val[1] != ' ': + raise QAPIParseError(self, "missing space after #") + return self.val[2:].rstrip() + + @staticmethod + def _match_at_name_colon(string: str) -> Optional[Match[str]]: + return re.match(r'@([^:]*): *', string) + + def get_doc_indented(self, doc: 'QAPIDoc') -> Optional[str]: + self.accept(False) + line = self.get_doc_line() + while line == '': + doc.append_line(line) + self.accept(False) + line = self.get_doc_line() + if line is None: + return line + indent = must_match(r'\s*', line).end() + if not indent: + return line + doc.append_line(line[indent:]) + prev_line_blank = False + while True: + self.accept(False) + line = self.get_doc_line() + if line is None: + return line + if self._match_at_name_colon(line): + return line + cur_indent = must_match(r'\s*', line).end() + if line != '' and cur_indent < indent: + if prev_line_blank: + return line + raise QAPIParseError( + self, + "unexpected de-indent (expected at least %d spaces)" % + indent) + doc.append_line(line[indent:]) + prev_line_blank = True + + def get_doc_paragraph(self, doc: 'QAPIDoc') -> Optional[str]: + while True: + self.accept(False) + line = self.get_doc_line() + if line is None: + return line + if line == '': + return line + doc.append_line(line) + + def get_doc(self) -> 'QAPIDoc': if self.val != '##': raise QAPIParseError( self, "junk after '##' at start of documentation comment") - - docs = [] - cur_doc = QAPIDoc(self, info) + info = self.info self.accept(False) - while self.tok == '#': - assert isinstance(self.val, str) - if self.val.startswith('##'): - # End of doc comment - if self.val != '##': - raise QAPIParseError( - self, - "junk after '##' at end of documentation comment") - cur_doc.end_comment() - docs.append(cur_doc) - self.accept() - return docs - if self.val.startswith('# ='): - if cur_doc.symbol: + line = self.get_doc_line() + if line is not None and line.startswith('@'): + # Definition documentation + if not line.endswith(':'): + raise QAPIParseError(self, "line should end with ':'") + # Invalid names are not checked here, but the name + # provided *must* match the following definition, + # which *is* validated in expr.py. + symbol = line[1:-1] + if not symbol: + raise QAPIParseError(self, "name required after '@'") + doc = QAPIDoc(self, info, symbol) + self.accept(False) + line = self.get_doc_line() + no_more_args = False + + while line is not None: + # Blank lines + while line == '': + self.accept(False) + line = self.get_doc_line() + if line is None: + break + # Non-blank line, first of a section + if line == 'Features:' and not doc.features: + self.accept(False) + line = self.get_doc_line() + while line == '': + self.accept(False) + line = self.get_doc_line() + while (line is not None + and (match := self._match_at_name_colon(line))): + doc.new_feature(match.group(1)) + text = line[match.end():] + if text: + doc.append_line(text) + line = self.get_doc_indented(doc) + no_more_args = True + elif match := self._match_at_name_colon(line): + # description + if no_more_args: + raise QAPIParseError( + self, + "description of '@%s:' follows a section" + % match.group(1)) + while (line is not None + and (match := self._match_at_name_colon(line))): + doc.new_argument(match.group(1)) + text = line[match.end():] + if text: + doc.append_line(text) + line = self.get_doc_indented(doc) + no_more_args = True + elif match := re.match( + r'(Returns|Since|Notes?|Examples?|TODO): *', + line): + # tagged section + doc.new_tagged_section(match.group(1)) + text = line[match.end():] + if text: + doc.append_line(text) + line = self.get_doc_indented(doc) + no_more_args = True + elif line.startswith('='): raise QAPIParseError( self, "unexpected '=' markup in definition documentation") - if cur_doc.body.text: + else: + # tag-less paragraph + doc.ensure_untagged_section() + doc.append_line(line) + line = self.get_doc_paragraph(doc) + else: + # Free-form documentation + doc = QAPIDoc(self, info) + doc.ensure_untagged_section() + first = True + while line is not None: + if match := self._match_at_name_colon(line): raise QAPIParseError( self, - "'=' heading must come first in a comment block") - cur_doc.append(self.val) - self.accept(False) + "'@%s:' not allowed in free-form documentation" + % match.group(1)) + if line.startswith('='): + if not first: + raise QAPIParseError( + self, + "'=' heading must come first in a comment block") + doc.append_line(line) + self.accept(False) + line = self.get_doc_line() + first = False - raise QAPIParseError(self, "documentation comment must end with '##'") + self.accept(False) + doc.end() + return doc class QAPIDoc: @@ -469,7 +601,6 @@ class QAPIDoc: """ class Section: - # pylint: disable=too-few-public-methods def __init__(self, parser: QAPISchemaParser, tag: Optional[str] = None): # section source info, i.e. where it begins @@ -480,29 +611,8 @@ class QAPIDoc: self.tag = tag # section text without tag self.text = '' - # indentation to strip (None means indeterminate) - self._indent = None if self.tag else 0 - - def append(self, line: str) -> None: - line = line.rstrip() - - if line: - indent = must_match(r'\s*', line).end() - if self._indent is None: - # indeterminate indentation - if self.text != '': - # non-blank, non-first line determines indentation - if indent == 0: - raise QAPIParseError( - self._parser, "line needs to be indented") - self._indent = indent - elif indent < self._indent: - raise QAPIParseError( - self._parser, - "unexpected de-indent (expected at least %d spaces)" % - self._indent) - line = line[self._indent:] + def append_line(self, line: str) -> None: self.text += line + '\n' class ArgSection(Section): @@ -514,243 +624,75 @@ class QAPIDoc: def connect(self, member: 'QAPISchemaMember') -> None: self.member = member - class NullSection(Section): - """ - Immutable dummy section for use at the end of a doc block. - """ - # pylint: disable=too-few-public-methods - def append(self, line: str) -> None: - assert False, "Text appended after end_comment() called." - - def __init__(self, parser: QAPISchemaParser, info: QAPISourceInfo): + def __init__(self, parser: QAPISchemaParser, info: QAPISourceInfo, + symbol: Optional[str] = None): # self._parser is used to report errors with QAPIParseError. The # resulting error position depends on the state of the parser. # It happens to be the beginning of the comment. More or less # servicable, but action at a distance. self._parser = parser + # info points to the doc comment block's first line self.info = info - self.symbol: Optional[str] = None - self.body = QAPIDoc.Section(parser) - # dicts mapping parameter/feature names to their ArgSection - self.args: Dict[str, QAPIDoc.ArgSection] = OrderedDict() - self.features: Dict[str, QAPIDoc.ArgSection] = OrderedDict() + # definition doc's symbol, None for free-form doc + self.symbol: Optional[str] = symbol + # the sections in textual order + self.all_sections: List[QAPIDoc.Section] = [QAPIDoc.Section(parser)] + # the body section + self.body: Optional[QAPIDoc.Section] = self.all_sections[0] + # dicts mapping parameter/feature names to their description + self.args: Dict[str, QAPIDoc.ArgSection] = {} + self.features: Dict[str, QAPIDoc.ArgSection] = {} + # sections other than .body, .args, .features self.sections: List[QAPIDoc.Section] = [] - # the current section - self._section = self.body - self._append_line = self._append_body_line - self._first_line_in_paragraph = False - - def has_section(self, tag: str) -> bool: - """Return True if we have a section with this tag.""" - for i in self.sections: - if i.tag == tag: - return True - return False - - def append(self, line: str) -> None: - """ - Parse a comment line and add it to the documentation. - - The way that the line is dealt with depends on which part of - the documentation we're parsing right now: - * The body section: ._append_line is ._append_body_line - * An argument section: ._append_line is ._append_args_line - * A features section: ._append_line is ._append_features_line - * An additional section: ._append_line is ._append_various_line - """ - line = line[1:] - if not line: - self._append_freeform(line) - self._first_line_in_paragraph = True - return - - if line[0] != ' ': - raise QAPIParseError(self._parser, "missing space after #") - line = line[1:] - self._append_line(line) - self._first_line_in_paragraph = False - - def end_comment(self) -> None: - self._switch_section(QAPIDoc.NullSection(self._parser)) - - @staticmethod - def _match_at_name_colon(string: str) -> Optional[Match[str]]: - return re.match(r'@([^:]*): *', string) - def _match_section_tag(self, string: str) -> Optional[Match[str]]: - if not self._first_line_in_paragraph: - return None - return re.match(r'(Returns|Since|Notes?|Examples?|TODO): *', - string) - - def _append_body_line(self, line: str) -> None: - """ - Process a line of documentation text in the body section. - - If this a symbol line and it is the section's first line, this - is a definition documentation block for that symbol. - - If it's a definition documentation block, another symbol line - begins the argument section for the argument named by it, and - a section tag begins an additional section. Start that - section and append the line to it. - - Else, append the line to the current section. - """ - # FIXME not nice: things like '# @foo:' and '# @foo: ' aren't - # recognized, and get silently treated as ordinary text - if not self.symbol and not self.body.text and line.startswith('@'): - if not line.endswith(':'): - raise QAPIParseError(self._parser, "line should end with ':'") - self.symbol = line[1:-1] - # Invalid names are not checked here, but the name provided MUST - # match the following definition, which *is* validated in expr.py. - if not self.symbol: - raise QAPIParseError( - self._parser, "name required after '@'") - elif self.symbol: - # This is a definition documentation block - if self._match_at_name_colon(line): - self._append_line = self._append_args_line - self._append_args_line(line) - elif line == 'Features:': - self._append_line = self._append_features_line - elif self._match_section_tag(line): - self._append_line = self._append_various_line - self._append_various_line(line) - else: - self._append_freeform(line) - else: - # This is a free-form documentation block - self._append_freeform(line) - - def _append_args_line(self, line: str) -> None: - """ - Process a line of documentation text in an argument section. - - A symbol line begins the next argument section, a section tag - section or a non-indented line after a blank line begins an - additional section. Start that section and append the line to - it. - - Else, append the line to the current section. - - """ - match = self._match_at_name_colon(line) - if match: - line = line[match.end():] - self._start_args_section(match.group(1)) - elif self._match_section_tag(line): - self._append_line = self._append_various_line - self._append_various_line(line) - return - elif (self._section.text.endswith('\n\n') - and line and not line[0].isspace()): - if line == 'Features:': - self._append_line = self._append_features_line - else: - self._start_section() - self._append_line = self._append_various_line - self._append_various_line(line) - return - - self._append_freeform(line) + def end(self) -> None: + for section in self.all_sections: + section.text = section.text.strip('\n') + if section.tag is not None and section.text == '': + raise QAPISemError( + section.info, "text required after '%s:'" % section.tag) - def _append_features_line(self, line: str) -> None: - match = self._match_at_name_colon(line) - if match: - line = line[match.end():] - self._start_features_section(match.group(1)) - elif self._match_section_tag(line): - self._append_line = self._append_various_line - self._append_various_line(line) - return - elif (self._section.text.endswith('\n\n') - and line and not line[0].isspace()): - self._start_section() - self._append_line = self._append_various_line - self._append_various_line(line) + def ensure_untagged_section(self) -> None: + if self.all_sections and not self.all_sections[-1].tag: + # extend current section + self.all_sections[-1].text += '\n' return + # start new section + section = self.Section(self._parser) + self.sections.append(section) + self.all_sections.append(section) + + def new_tagged_section(self, tag: str) -> None: + if tag in ('Returns', 'Since'): + for section in self.all_sections: + if isinstance(section, self.ArgSection): + continue + if section.tag == tag: + raise QAPIParseError( + self._parser, "duplicated '%s' section" % tag) + section = self.Section(self._parser, tag) + self.sections.append(section) + self.all_sections.append(section) - self._append_freeform(line) - - def _append_various_line(self, line: str) -> None: - """ - Process a line of documentation text in an additional section. - - A symbol line is an error. - - A section tag begins an additional section. Start that - section and append the line to it. - - Else, append the line to the current section. - """ - match = self._match_at_name_colon(line) - if match: - raise QAPIParseError(self._parser, - "description of '@%s:' follows a section" - % match.group(1)) - match = self._match_section_tag(line) - if match: - line = line[match.end():] - self._start_section(match.group(1)) - - self._append_freeform(line) - - def _start_symbol_section( - self, - symbols_dict: Dict[str, 'QAPIDoc.ArgSection'], - name: str) -> None: - # FIXME invalid names other than the empty string aren't flagged + def _new_description(self, name: str, + desc: Dict[str, ArgSection]) -> None: if not name: raise QAPIParseError(self._parser, "invalid parameter name") - if name in symbols_dict: + if name in desc: raise QAPIParseError(self._parser, "'%s' parameter name duplicated" % name) - assert not self.sections - new_section = QAPIDoc.ArgSection(self._parser, '@' + name) - self._switch_section(new_section) - symbols_dict[name] = new_section - - def _start_args_section(self, name: str) -> None: - self._start_symbol_section(self.args, name) - - def _start_features_section(self, name: str) -> None: - self._start_symbol_section(self.features, name) + section = self.ArgSection(self._parser, '@' + name) + self.all_sections.append(section) + desc[name] = section - def _start_section(self, tag: Optional[str] = None) -> None: - if not tag and not self._section.tag: - # extend current section - return - if tag in ('Returns', 'Since') and self.has_section(tag): - raise QAPIParseError(self._parser, - "duplicated '%s' section" % tag) - new_section = QAPIDoc.Section(self._parser, tag) - self._switch_section(new_section) - self.sections.append(new_section) - - def _switch_section(self, new_section: 'QAPIDoc.Section') -> None: - text = self._section.text = self._section.text.strip('\n') - - # Only the 'body' section is allowed to have an empty body. - # All other sections, including anonymous ones, must have text. - if self._section != self.body and not text: - # We do not create anonymous sections unless there is - # something to put in them; this is a parser bug. - assert self._section.tag - raise QAPISemError( - self._section.info, - "text required after '%s:'" % self._section.tag) + def new_argument(self, name: str) -> None: + self._new_description(name, self.args) - self._section = new_section + def new_feature(self, name: str) -> None: + self._new_description(name, self.features) - def _append_freeform(self, line: str) -> None: - match = re.match(r'(@\S+:)', line) - if match: - raise QAPIParseError(self._parser, - "'%s' not allowed in free-form documentation" - % match.group(1)) - self._section.append(line) + def append_line(self, line: str) -> None: + self.all_sections[-1].append_line(line) def connect_member(self, member: 'QAPISchemaMember') -> None: if member.name not in self.args: @@ -758,8 +700,8 @@ class QAPIDoc: raise QAPISemError(member.info, "%s '%s' lacks documentation" % (member.role, member.name)) - self.args[member.name] = QAPIDoc.ArgSection(self._parser, - '@' + member.name) + self.args[member.name] = QAPIDoc.ArgSection( + self._parser, '@' + member.name) self.args[member.name].connect(member) def connect_feature(self, feature: 'QAPISchemaFeature') -> None: -- cgit v1.1 From 629c5075aa6fb853855256cd7d380903e9b7ffbc Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 16 Feb 2024 15:58:39 +0100 Subject: qapi: Reject multiple and empty feature descriptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parser recognizes only the first "Features:" line. Any subsequent ones are treated as ordinary text, as visible in test case doc-duplicate-features. Recognize "Features:" lines anywhere. A second one is an error. A 'Features:' line without any features is useless, but not an error. Make it an error. This makes detecting a second "Features:" line easier. qapi/run-state.json actually has an instance of this since commit fe17522d854 (qapi: Remove deprecated 'singlestep' member of StatusInfo). Clean it up. Signed-off-by: Markus Armbruster Message-ID: <20240216145841.2099240-16-armbru@redhat.com> Reviewed-by: Daniel P. Berrangé --- scripts/qapi/parser.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 73ff150..3d8c62b 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -507,7 +507,10 @@ class QAPISchemaParser: if line is None: break # Non-blank line, first of a section - if line == 'Features:' and not doc.features: + if line == 'Features:': + if doc.features: + raise QAPIParseError( + self, "duplicated 'Features:' line") self.accept(False) line = self.get_doc_line() while line == '': @@ -520,6 +523,9 @@ class QAPISchemaParser: if text: doc.append_line(text) line = self.get_doc_indented(doc) + if not doc.features: + raise QAPIParseError( + self, 'feature descriptions expected') no_more_args = True elif match := self._match_at_name_colon(line): # description -- cgit v1.1 From adb0193b90bd1fecd7d6dda70fc1c2d2e45ceae0 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 16 Feb 2024 15:58:40 +0100 Subject: qapi: Divorce QAPIDoc from QAPIParseError MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QAPIDoc stores a reference to QAPIParser just to pass it to QAPIParseError. The resulting error position depends on the state of the parser. It happens to be the current comment line. Servicable, but action at a distance. The commit before previous moved most uses of QAPIParseError from QAPIDoc to QAPIParser. There are just three left. Convert them to QAPISemError. This involves passing info to a few methods. Then drop the reference to QAPIParser. The three errors lose the column number. Not really interesting here: it's the comment line's indentation. Signed-off-by: Markus Armbruster Message-ID: <20240216145841.2099240-17-armbru@redhat.com> Reviewed-by: Daniel P. Berrangé --- scripts/qapi/parser.py | 66 +++++++++++++++++++++----------------------------- 1 file changed, 28 insertions(+), 38 deletions(-) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 3d8c62b..1170741 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -494,7 +494,7 @@ class QAPISchemaParser: symbol = line[1:-1] if not symbol: raise QAPIParseError(self, "name required after '@'") - doc = QAPIDoc(self, info, symbol) + doc = QAPIDoc(info, symbol) self.accept(False) line = self.get_doc_line() no_more_args = False @@ -518,7 +518,7 @@ class QAPISchemaParser: line = self.get_doc_line() while (line is not None and (match := self._match_at_name_colon(line))): - doc.new_feature(match.group(1)) + doc.new_feature(self.info, match.group(1)) text = line[match.end():] if text: doc.append_line(text) @@ -536,7 +536,7 @@ class QAPISchemaParser: % match.group(1)) while (line is not None and (match := self._match_at_name_colon(line))): - doc.new_argument(match.group(1)) + doc.new_argument(self.info, match.group(1)) text = line[match.end():] if text: doc.append_line(text) @@ -546,7 +546,7 @@ class QAPISchemaParser: r'(Returns|Since|Notes?|Examples?|TODO): *', line): # tagged section - doc.new_tagged_section(match.group(1)) + doc.new_tagged_section(self.info, match.group(1)) text = line[match.end():] if text: doc.append_line(text) @@ -558,13 +558,13 @@ class QAPISchemaParser: "unexpected '=' markup in definition documentation") else: # tag-less paragraph - doc.ensure_untagged_section() + doc.ensure_untagged_section(self.info) doc.append_line(line) line = self.get_doc_paragraph(doc) else: # Free-form documentation - doc = QAPIDoc(self, info) - doc.ensure_untagged_section() + doc = QAPIDoc(info) + doc.ensure_untagged_section(self.info) first = True while line is not None: if match := self._match_at_name_colon(line): @@ -607,12 +607,10 @@ class QAPIDoc: """ class Section: - def __init__(self, parser: QAPISchemaParser, + def __init__(self, info: QAPISourceInfo, tag: Optional[str] = None): # section source info, i.e. where it begins - self.info = parser.info - # parser, for error messages about indentation - self._parser = parser + self.info = info # section tag, if any ('Returns', '@name', ...) self.tag = tag # section text without tag @@ -622,27 +620,20 @@ class QAPIDoc: self.text += line + '\n' class ArgSection(Section): - def __init__(self, parser: QAPISchemaParser, - tag: str): - super().__init__(parser, tag) + def __init__(self, info: QAPISourceInfo, tag: str): + super().__init__(info, tag) self.member: Optional['QAPISchemaMember'] = None def connect(self, member: 'QAPISchemaMember') -> None: self.member = member - def __init__(self, parser: QAPISchemaParser, info: QAPISourceInfo, - symbol: Optional[str] = None): - # self._parser is used to report errors with QAPIParseError. The - # resulting error position depends on the state of the parser. - # It happens to be the beginning of the comment. More or less - # servicable, but action at a distance. - self._parser = parser + def __init__(self, info: QAPISourceInfo, symbol: Optional[str] = None): # info points to the doc comment block's first line self.info = info # definition doc's symbol, None for free-form doc self.symbol: Optional[str] = symbol # the sections in textual order - self.all_sections: List[QAPIDoc.Section] = [QAPIDoc.Section(parser)] + self.all_sections: List[QAPIDoc.Section] = [QAPIDoc.Section(info)] # the body section self.body: Optional[QAPIDoc.Section] = self.all_sections[0] # dicts mapping parameter/feature names to their description @@ -658,44 +649,43 @@ class QAPIDoc: raise QAPISemError( section.info, "text required after '%s:'" % section.tag) - def ensure_untagged_section(self) -> None: + def ensure_untagged_section(self, info: QAPISourceInfo) -> None: if self.all_sections and not self.all_sections[-1].tag: # extend current section self.all_sections[-1].text += '\n' return # start new section - section = self.Section(self._parser) + section = self.Section(info) self.sections.append(section) self.all_sections.append(section) - def new_tagged_section(self, tag: str) -> None: + def new_tagged_section(self, info: QAPISourceInfo, tag: str) -> None: if tag in ('Returns', 'Since'): for section in self.all_sections: if isinstance(section, self.ArgSection): continue if section.tag == tag: - raise QAPIParseError( - self._parser, "duplicated '%s' section" % tag) - section = self.Section(self._parser, tag) + raise QAPISemError( + info, "duplicated '%s' section" % tag) + section = self.Section(info, tag) self.sections.append(section) self.all_sections.append(section) - def _new_description(self, name: str, + def _new_description(self, info: QAPISourceInfo, name: str, desc: Dict[str, ArgSection]) -> None: if not name: - raise QAPIParseError(self._parser, "invalid parameter name") + raise QAPISemError(info, "invalid parameter name") if name in desc: - raise QAPIParseError(self._parser, - "'%s' parameter name duplicated" % name) - section = self.ArgSection(self._parser, '@' + name) + raise QAPISemError(info, "'%s' parameter name duplicated" % name) + section = self.ArgSection(info, '@' + name) self.all_sections.append(section) desc[name] = section - def new_argument(self, name: str) -> None: - self._new_description(name, self.args) + def new_argument(self, info: QAPISourceInfo, name: str) -> None: + self._new_description(info, name, self.args) - def new_feature(self, name: str) -> None: - self._new_description(name, self.features) + def new_feature(self, info: QAPISourceInfo, name: str) -> None: + self._new_description(info, name, self.features) def append_line(self, line: str) -> None: self.all_sections[-1].append_line(line) @@ -707,7 +697,7 @@ class QAPIDoc: "%s '%s' lacks documentation" % (member.role, member.name)) self.args[member.name] = QAPIDoc.ArgSection( - self._parser, '@' + member.name) + self.info, '@' + member.name) self.args[member.name].connect(member) def connect_feature(self, feature: 'QAPISchemaFeature') -> None: -- cgit v1.1