From 5b5fe0e018891dc08bcce75f064015c42aeb46a4 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 28 Apr 2023 12:54:15 +0200 Subject: qapi: Fix crash on stray double quote character When the lexer chokes on a stray character, its shows the characters until the next structural character in the error message. It uses a regular expression to match a non-empty string of non-structural characters. Bug: the regular expression treats '"' as structural. When the lexer chokes on '"', the match fails, and trips must_match()'s assertion. Fix the regular expression. Fixes: 14c32795024c (qapi: Improve reporting of lexical errors) Signed-off-by: Markus Armbruster Message-Id: <20230428105429.1687850-4-armbru@redhat.com> Reviewed-by: Juan Quintela --- scripts/qapi/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 878f90b458..7b49d3ab05 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -346,7 +346,7 @@ class QAPISchemaParser: elif not self.tok.isspace(): # Show up to next structural, whitespace or quote # character - match = must_match('[^[\\]{}:,\\s\'"]+', + match = must_match('[^[\\]{}:,\\s\']+', self.src[self.cursor-1:]) raise QAPIParseError(self, "stray '%s'" % match.group(0)) -- cgit 1.4.1 From 9b2c6746d30a44d222e9124faee59eb05703b6ae Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 28 Apr 2023 12:54:23 +0200 Subject: qapi: Fix argument description indentation stripping When an argument's description starts on the line after the "#arg: " line, indentation is stripped only from the description's first line, as demonstrated by the previous commit. Moreover, subsequent lines with less indentation are not rejected. Make the first line's indentation the expected indentation for the remainder of the description. This fixes indentation stripping, and also requires at least that much indentation. Signed-off-by: Markus Armbruster Message-Id: <20230428105429.1687850-12-armbru@redhat.com> Reviewed-by: Juan Quintela --- scripts/qapi/parser.py | 20 +++++++++++--------- tests/qapi-schema/doc-good.out | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 7b49d3ab05..ddc14ceaba 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -483,7 +483,9 @@ class QAPIDoc: # Blank lines are always OK. if line: indent = must_match(r'\s*', line).end() - if indent < self._indent: + if self._indent < 0: + self._indent = indent + elif indent < self._indent: raise QAPIParseError( self._parser, "unexpected de-indent (expected at least %d spaces)" % @@ -631,9 +633,9 @@ class QAPIDoc: indent = must_match(r'@\S*:\s*', line).end() line = line[indent:] if not line: - # Line was just the "@arg:" header; following lines - # are not indented - indent = 0 + # Line was just the "@arg:" header + # The next non-blank line determines expected indent + indent = -1 else: line = ' ' * indent + line self._start_args_section(name[1:-1], indent) @@ -666,9 +668,9 @@ class QAPIDoc: indent = must_match(r'@\S*:\s*', line).end() line = line[indent:] if not line: - # Line was just the "@arg:" header; following lines - # are not indented - indent = 0 + # Line was just the "@arg:" header + # The next non-blank line determines expected indent + indent = -1 else: line = ' ' * indent + line self._start_features_section(name[1:-1], indent) @@ -712,8 +714,8 @@ class QAPIDoc: indent = must_match(r'\S*:\s*', line).end() line = line[indent:] if not line: - # Line was just the "Section:" header; following lines - # are not indented + # Line was just the "Section:" header + # The next non-blank line determines expected indent indent = 0 else: line = ' ' * indent + line diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out index 2ba72ae558..277371acc8 100644 --- a/tests/qapi-schema/doc-good.out +++ b/tests/qapi-schema/doc-good.out @@ -159,7 +159,7 @@ doc symbol=cmd arg=arg1 description starts on a new line, - indented +indented arg=arg2 the second argument arg=arg3 -- cgit 1.4.1 From 3e32dca3f0d1eddcfecf963d94b9ff60e3e08448 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 28 Apr 2023 12:54:24 +0200 Subject: qapi: Rewrite parsing of doc comment section symbols and tags To recognize a line starting with a section symbol and or tag, we first split it at the first space, then examine the part left of the space. We can just as well examine the unsplit line, so do that. Signed-off-by: Markus Armbruster Message-Id: <20230428105429.1687850-13-armbru@redhat.com> Reviewed-by: Juan Quintela [Work around lack of walrus operator in Python 3.7 and older] --- scripts/qapi/parser.py | 55 +++++++++++++++++++++++++------------------------- 1 file changed, 27 insertions(+), 28 deletions(-) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index ddc14ceaba..a4ff9b6dbf 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -560,12 +560,12 @@ class QAPIDoc: self._switch_section(QAPIDoc.NullSection(self._parser)) @staticmethod - def _is_section_tag(name: str) -> bool: - return name in ('Returns:', 'Since:', - # those are often singular or plural - 'Note:', 'Notes:', - 'Example:', 'Examples:', - 'TODO:') + def _match_at_name_colon(string: str) -> re.Match: + return re.match(r'@([^:]*): *', string) + + @staticmethod + def _match_section_tag(string: str) -> re.Match: + return re.match(r'(Returns|Since|Notes?|Examples?|TODO): *', string) def _append_body_line(self, line: str) -> None: """ @@ -581,7 +581,6 @@ class QAPIDoc: Else, append the line to the current section. """ - name = line.split(' ', 1)[0] # 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('@'): @@ -595,12 +594,12 @@ class QAPIDoc: self._parser, "name required after '@'") elif self.symbol: # This is a definition documentation block - if name.startswith('@') and name.endswith(':'): + 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._is_section_tag(name): + elif self._match_section_tag(line): self._append_line = self._append_various_line self._append_various_line(line) else: @@ -621,16 +620,16 @@ class QAPIDoc: Else, append the line to the current section. """ - name = line.split(' ', 1)[0] - - if name.startswith('@') and name.endswith(':'): + match = self._match_at_name_colon(line) + if match: # If line is "@arg: first line of description", find # the index of 'f', which is the indent we expect for any # following lines. We then remove the leading "@arg:" # from line and replace it with spaces so that 'f' has the # same index as it did in the original line and can be # handled the same way we will handle following lines. - indent = must_match(r'@\S*:\s*', line).end() + name = match.group(1) + indent = match.end() line = line[indent:] if not line: # Line was just the "@arg:" header @@ -638,8 +637,8 @@ class QAPIDoc: indent = -1 else: line = ' ' * indent + line - self._start_args_section(name[1:-1], indent) - elif self._is_section_tag(name): + self._start_args_section(name, indent) + elif self._match_section_tag(line): self._append_line = self._append_various_line self._append_various_line(line) return @@ -656,16 +655,16 @@ class QAPIDoc: self._append_freeform(line) def _append_features_line(self, line: str) -> None: - name = line.split(' ', 1)[0] - - if name.startswith('@') and name.endswith(':'): + match = self._match_at_name_colon(line) + if match: # If line is "@arg: first line of description", find # the index of 'f', which is the indent we expect for any # following lines. We then remove the leading "@arg:" # from line and replace it with spaces so that 'f' has the # same index as it did in the original line and can be # handled the same way we will handle following lines. - indent = must_match(r'@\S*:\s*', line).end() + name = match.group(1) + indent = match.end() line = line[indent:] if not line: # Line was just the "@arg:" header @@ -673,8 +672,8 @@ class QAPIDoc: indent = -1 else: line = ' ' * indent + line - self._start_features_section(name[1:-1], indent) - elif self._is_section_tag(name): + self._start_features_section(name, indent) + elif self._match_section_tag(line): self._append_line = self._append_various_line self._append_various_line(line) return @@ -698,13 +697,13 @@ class QAPIDoc: Else, append the line to the current section. """ - name = line.split(' ', 1)[0] - - if name.startswith('@') and name.endswith(':'): + match = self._match_at_name_colon(line) + if match: raise QAPIParseError(self._parser, - "'%s' can't follow '%s' section" - % (name, self.sections[0].name)) - if self._is_section_tag(name): + "'@%s:' can't follow '%s' section" + % (match.group(1), self.sections[0].name)) + match = self._match_section_tag(line) + if match: # If line is "Section: first line of description", find # the index of 'f', which is the indent we expect for any # following lines. We then remove the leading "Section:" @@ -719,7 +718,7 @@ class QAPIDoc: indent = 0 else: line = ' ' * indent + line - self._start_section(name[:-1], indent) + self._start_section(match.group(1), indent) self._append_freeform(line) -- cgit 1.4.1 From 08349786c84306863a3b659c8a9b28bb74c405c6 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 28 Apr 2023 12:54:25 +0200 Subject: qapi: Relax doc string @name: description indentation rules The QAPI schema doc comment language provides special syntax for command and event arguments, struct and union members, alternate branches, enumeration values, and features: descriptions starting with "@name:". By convention, we format them like this: # @name: Lorem ipsum dolor sit amet, consectetur adipiscing elit, # sed do eiusmod tempor incididunt ut labore et dolore # magna aliqua. Okay for names as short as "name", but we have much longer ones. Their description gets squeezed against the right margin, like this: # @dirty-sync-missed-zero-copy: Number of times dirty RAM synchronization could # not avoid copying dirty pages. This is between # 0 and @dirty-sync-count * @multifd-channels. # (since 7.1) The description text is effectively just 50 characters wide. Easy enough to read, but can be cumbersome to write. The awkward squeeze against the right margin makes people go beyond it, which produces two undesirables: arguments about style, and descriptions that are unnecessarily hard to read, like this one: # @postcopy-vcpu-blocktime: list of the postcopy blocktime per vCPU. This is # only present when the postcopy-blocktime migration capability # is enabled. (Since 3.0) We could instead format it like # @postcopy-vcpu-blocktime: # list of the postcopy blocktime per vCPU. This is only present # when the postcopy-blocktime migration capability is # enabled. (Since 3.0) or, since the commit before previous, like # @postcopy-vcpu-blocktime: # list of the postcopy blocktime per vCPU. This is only present # when the postcopy-blocktime migration capability is # enabled. (Since 3.0) However, I'd rather have # @postcopy-vcpu-blocktime: list of the postcopy blocktime per vCPU. # This is only present when the postcopy-blocktime migration # capability is enabled. (Since 3.0) because this is how rST field and option lists work. To get this, we need to let the first non-blank line after the "@name:" line determine expected indentation. This fills up the indentation pitfall mentioned in docs/devel/qapi-code-gen.rst. A related pitfall still exists. Update the text to show it. Signed-off-by: Markus Armbruster Message-Id: <20230428105429.1687850-14-armbru@redhat.com> Reviewed-by: Juan Quintela [Work around lack of walrus operator in Python 3.7 and older] --- docs/devel/qapi-code-gen.rst | 10 +++-- scripts/qapi/parser.py | 73 +++++++++-------------------------- tests/qapi-schema/doc-bad-indent.err | 2 +- tests/qapi-schema/doc-bad-indent.json | 3 +- tests/qapi-schema/doc-good.json | 3 +- tests/qapi-schema/doc-good.out | 3 +- 6 files changed, 32 insertions(+), 62 deletions(-) (limited to 'scripts/qapi/parser.py') diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst index 6386b58881..875f893be2 100644 --- a/docs/devel/qapi-code-gen.rst +++ b/docs/devel/qapi-code-gen.rst @@ -1074,10 +1074,14 @@ Indentation matters. Bad example:: # @none: None (no memory side cache in this proximity domain, # or cache associativity unknown) + # (since 5.0) -The description is parsed as a definition list with term "None (no -memory side cache in this proximity domain," and definition "or cache -associativity unknown)". +The last line's de-indent is wrong. The second and subsequent lines +need to line up with each other, like this:: + + # @none: None (no memory side cache in this proximity domain, + # or cache associativity unknown) + # (since 5.0) Section tags are case-sensitive and end with a colon. Good example:: diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index a4ff9b6dbf..285c9ff4c9 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -474,17 +474,21 @@ class QAPIDoc: self._parser = parser # optional section name (argument/member or section name) self.name = name + # section text without section name self.text = '' - # the expected indent level of the text of this section - self._indent = indent + # indentation to strip (None means indeterminate) + self._indent = None if self.name else 0 def append(self, line: str) -> None: - # Strip leading spaces corresponding to the expected indent level - # Blank lines are always OK. + line = line.rstrip() + if line: indent = must_match(r'\s*', line).end() - if self._indent < 0: - self._indent = indent + if self._indent is None: + # indeterminate indentation + if self.text != '': + # non-blank, non-first line determines indentation + self._indent = indent elif indent < self._indent: raise QAPIParseError( self._parser, @@ -492,7 +496,7 @@ class QAPIDoc: self._indent) line = line[self._indent:] - self.text += line.rstrip() + '\n' + self.text += line + '\n' class ArgSection(Section): def __init__(self, parser: QAPISchemaParser, @@ -622,22 +626,8 @@ class QAPIDoc: """ match = self._match_at_name_colon(line) if match: - # If line is "@arg: first line of description", find - # the index of 'f', which is the indent we expect for any - # following lines. We then remove the leading "@arg:" - # from line and replace it with spaces so that 'f' has the - # same index as it did in the original line and can be - # handled the same way we will handle following lines. - name = match.group(1) - indent = match.end() - line = line[indent:] - if not line: - # Line was just the "@arg:" header - # The next non-blank line determines expected indent - indent = -1 - else: - line = ' ' * indent + line - self._start_args_section(name, indent) + line = line[match.end():] + self._start_args_section(match.group(1), 0) elif self._match_section_tag(line): self._append_line = self._append_various_line self._append_various_line(line) @@ -657,22 +647,8 @@ class QAPIDoc: def _append_features_line(self, line: str) -> None: match = self._match_at_name_colon(line) if match: - # If line is "@arg: first line of description", find - # the index of 'f', which is the indent we expect for any - # following lines. We then remove the leading "@arg:" - # from line and replace it with spaces so that 'f' has the - # same index as it did in the original line and can be - # handled the same way we will handle following lines. - name = match.group(1) - indent = match.end() - line = line[indent:] - if not line: - # Line was just the "@arg:" header - # The next non-blank line determines expected indent - indent = -1 - else: - line = ' ' * indent + line - self._start_features_section(name, indent) + line = line[match.end():] + self._start_features_section(match.group(1), 0) elif self._match_section_tag(line): self._append_line = self._append_various_line self._append_various_line(line) @@ -704,21 +680,8 @@ class QAPIDoc: % (match.group(1), self.sections[0].name)) match = self._match_section_tag(line) if match: - # If line is "Section: first line of description", find - # the index of 'f', which is the indent we expect for any - # following lines. We then remove the leading "Section:" - # from line and replace it with spaces so that 'f' has the - # same index as it did in the original line and can be - # handled the same way we will handle following lines. - indent = must_match(r'\S*:\s*', line).end() - line = line[indent:] - if not line: - # Line was just the "Section:" header - # The next non-blank line determines expected indent - indent = 0 - else: - line = ' ' * indent + line - self._start_section(match.group(1), indent) + line = line[match.end():] + self._start_section(match.group(1), 0) self._append_freeform(line) @@ -754,7 +717,7 @@ class QAPIDoc: self.sections.append(new_section) def _switch_section(self, new_section: 'QAPIDoc.Section') -> None: - text = self._section.text = self._section.text.strip() + 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. diff --git a/tests/qapi-schema/doc-bad-indent.err b/tests/qapi-schema/doc-bad-indent.err index 67844539bd..3c9699a8e0 100644 --- a/tests/qapi-schema/doc-bad-indent.err +++ b/tests/qapi-schema/doc-bad-indent.err @@ -1 +1 @@ -doc-bad-indent.json:6:1: unexpected de-indent (expected at least 4 spaces) +doc-bad-indent.json:7:1: unexpected de-indent (expected at least 2 spaces) diff --git a/tests/qapi-schema/doc-bad-indent.json b/tests/qapi-schema/doc-bad-indent.json index edde8f21dc..3f22a27717 100644 --- a/tests/qapi-schema/doc-bad-indent.json +++ b/tests/qapi-schema/doc-bad-indent.json @@ -3,6 +3,7 @@ ## # @foo: # @a: line one -# line two is wrongly indented +# line two +# line three is wrongly indented ## { 'command': 'foo', 'data': { 'a': 'int' } } diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json index 34c3dcbe97..354dfdf461 100644 --- a/tests/qapi-schema/doc-good.json +++ b/tests/qapi-schema/doc-good.json @@ -144,7 +144,8 @@ # description starts on a new line, # indented # -# @arg2: the second argument +# @arg2: description starts on the same line +# remainder indented differently # # Features: # @cmd-feat1: a feature diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out index 277371acc8..24d9ea954d 100644 --- a/tests/qapi-schema/doc-good.out +++ b/tests/qapi-schema/doc-good.out @@ -161,7 +161,8 @@ doc symbol=cmd description starts on a new line, indented arg=arg2 -the second argument +description starts on the same line +remainder indented differently arg=arg3 feature=cmd-feat1 -- cgit 1.4.1 From eb59cf76285cb54caba246383821cd7ee2b0f539 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 28 Apr 2023 12:54:26 +0200 Subject: qapi: Section parameter @indent is no longer used, drop Signed-off-by: Markus Armbruster Message-Id: <20230428105429.1687850-15-armbru@redhat.com> Reviewed-by: Juan Quintela --- scripts/qapi/parser.py | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) (limited to 'scripts/qapi/parser.py') diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 285c9ff4c9..4923a59d60 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -468,8 +468,7 @@ class QAPIDoc: class Section: # pylint: disable=too-few-public-methods def __init__(self, parser: QAPISchemaParser, - name: Optional[str] = None, indent: int = 0): - + name: Optional[str] = None): # parser, for error messages about indentation self._parser = parser # optional section name (argument/member or section name) @@ -500,8 +499,8 @@ class QAPIDoc: class ArgSection(Section): def __init__(self, parser: QAPISchemaParser, - name: str, indent: int = 0): - super().__init__(parser, name, indent) + name: str): + super().__init__(parser, name) self.member: Optional['QAPISchemaMember'] = None def connect(self, member: 'QAPISchemaMember') -> None: @@ -627,7 +626,7 @@ class QAPIDoc: match = self._match_at_name_colon(line) if match: line = line[match.end():] - self._start_args_section(match.group(1), 0) + 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) @@ -648,7 +647,7 @@ class QAPIDoc: match = self._match_at_name_colon(line) if match: line = line[match.end():] - self._start_features_section(match.group(1), 0) + 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) @@ -681,15 +680,14 @@ class QAPIDoc: match = self._match_section_tag(line) if match: line = line[match.end():] - self._start_section(match.group(1), 0) + self._start_section(match.group(1)) self._append_freeform(line) def _start_symbol_section( self, symbols_dict: Dict[str, 'QAPIDoc.ArgSection'], - name: str, - indent: int) -> None: + name: str) -> None: # FIXME invalid names other than the empty string aren't flagged if not name: raise QAPIParseError(self._parser, "invalid parameter name") @@ -697,22 +695,21 @@ class QAPIDoc: raise QAPIParseError(self._parser, "'%s' parameter name duplicated" % name) assert not self.sections - new_section = QAPIDoc.ArgSection(self._parser, name, indent) + new_section = QAPIDoc.ArgSection(self._parser, name) self._switch_section(new_section) symbols_dict[name] = new_section - def _start_args_section(self, name: str, indent: int) -> None: - self._start_symbol_section(self.args, name, indent) + def _start_args_section(self, name: str) -> None: + self._start_symbol_section(self.args, name) - def _start_features_section(self, name: str, indent: int) -> None: - self._start_symbol_section(self.features, name, indent) + def _start_features_section(self, name: str) -> None: + self._start_symbol_section(self.features, name) - def _start_section(self, name: Optional[str] = None, - indent: int = 0) -> None: + def _start_section(self, name: Optional[str] = None) -> None: if name in ('Returns', 'Since') and self.has_section(name): raise QAPIParseError(self._parser, "duplicated '%s' section" % name) - new_section = QAPIDoc.Section(self._parser, name, indent) + new_section = QAPIDoc.Section(self._parser, name) self._switch_section(new_section) self.sections.append(new_section) -- cgit 1.4.1