From c880a8da9c089cb35e7018a77d388bcf7f0d8708 Mon Sep 17 00:00:00 2001 From: Andres Toom Date: Mon, 14 Oct 2024 21:47:32 +0300 Subject: [PATCH] Make integer syntax style check configurable Ref. eng/recordflux/RecordFlux#1775 --- doc/user_guide/90-appendix.rst | 17 ++- rflx/const.py | 17 +++ rflx/model/model.py | 2 + rflx/model/top_level_declaration.py | 9 ++ rflx/model/type_decl.py | 79 ++++++----- rflx/specification/parser.py | 30 ++++- rflx/specification/style.py | 171 +++++++++++++++--------- tests/end_to_end/cli_test.py | 62 +++++++++ tests/integration/cli_test.py | 102 +++++++++++++- tests/tools/check_doc_test.py | 4 +- tests/unit/ls/lexer_test.py | 2 +- tests/unit/ls/server_test.py | 17 ++- tests/unit/model/message_test.py | 6 +- tests/unit/model/model_test.py | 6 +- tests/unit/model/type_decl_test.py | 69 ++++++++-- tests/unit/specification/parser_test.py | 2 +- tests/unit/specification/style_test.py | 141 +++++++++++++++---- tools/check_doc.py | 3 +- 18 files changed, 584 insertions(+), 155 deletions(-) diff --git a/doc/user_guide/90-appendix.rst b/doc/user_guide/90-appendix.rst index d88a8107f..af45aed7c 100644 --- a/doc/user_guide/90-appendix.rst +++ b/doc/user_guide/90-appendix.rst @@ -51,8 +51,9 @@ Style Checks ^^^^^^^^^^^^ By default, the style of specification files is checked. +Error messages about style violations have a style check identifier appended to the message in the following form "[style:]". +For example: "[style:line-length]". Style checks can be disabled for individual files by adding a pragma to the first line of the file. -Besides the deactivation of specific checks, it is also possible to disable all checks by using ``all``. **Example** @@ -65,6 +66,20 @@ Besides the deactivation of specific checks, it is also possible to disable all end P; +It is also possible to disable all style checks by using the identifier ``all``. + +**Example** + +.. doc-check: rflx +.. code:: rflx + + -- style: disable = all + + package P is + + end P; + + Integration Files ^^^^^^^^^^^^^^^^^ diff --git a/rflx/const.py b/rflx/const.py index 731c71bb1..01793528f 100644 --- a/rflx/const.py +++ b/rflx/const.py @@ -1,4 +1,5 @@ import multiprocessing +from enum import Enum from pathlib import Path from typing import Final @@ -15,6 +16,22 @@ # RecordFlux is executed by another process, e.g., when the language server is started by VS Code. MP_CONTEXT = multiprocessing.get_context("forkserver") + +class StyleCheck(Enum): + ALL = "all" + BLANK_LINES = "blank-lines" + CHARACTERS = "characters" + INDENTATION = "indentation" + INTEGER_SYNTAX = "integer-syntax" + LINE_LENGTH = "line-length" + TOKEN_SPACING = "token-spacing" + TRAILING_SPACES = "trailing-spaces" + + +MODEL_STYLE_CHECKS: Final = frozenset({StyleCheck.INTEGER_SYNTAX}) +BASIC_STYLE_CHECKS: Final = frozenset(set(StyleCheck) - MODEL_STYLE_CHECKS) + + RESERVED_WORDS = [ # Ada "abort", diff --git a/rflx/model/model.py b/rflx/model/model.py index ac29886d2..bf28422ad 100644 --- a/rflx/model/model.py +++ b/rflx/model/model.py @@ -19,6 +19,7 @@ @dataclass class UncheckedModel(Base): declarations: Sequence[top_level_declaration.UncheckedTopLevelDeclaration] + style_checks: dict[Path, frozenset[const.StyleCheck]] error: RecordFluxError def checked( @@ -43,6 +44,7 @@ def checked( else: logging.info("Verifying {identifier}", identifier=d.identifier) checked = d.checked(declarations, workers=workers) + checked.check_style(error, self.style_checks) declarations.append(checked) cache.add_verified(digest) except RecordFluxError as e: # noqa: PERF203 diff --git a/rflx/model/top_level_declaration.py b/rflx/model/top_level_declaration.py index 4d5935617..224d46750 100644 --- a/rflx/model/top_level_declaration.py +++ b/rflx/model/top_level_declaration.py @@ -3,7 +3,9 @@ from abc import abstractmethod from collections.abc import Sequence from dataclasses import dataclass +from pathlib import Path +from rflx import const from rflx.common import Base from rflx.identifier import ID, StrID from rflx.rapidflux import NO_LOCATION, ErrorEntry, Location, RecordFluxError, Severity @@ -32,6 +34,13 @@ def name(self) -> str: def package(self) -> ID: return self.identifier.parent + def check_style( + self, + error: RecordFluxError, + style_checks: dict[Path, frozenset[const.StyleCheck]], + ) -> None: + pass + def _check_identifier(self) -> None: if len(self.identifier.parts) != 2: self.error.extend( diff --git a/rflx/model/type_decl.py b/rflx/model/type_decl.py index de1fb357b..c1a024a22 100644 --- a/rflx/model/type_decl.py +++ b/rflx/model/type_decl.py @@ -104,14 +104,13 @@ def constraints( class Integer(Scalar): - def __init__( # noqa: PLR0913 + def __init__( self, identifier: StrID, first: expr.Expr, last: expr.Expr, size: expr.Expr, location: Location = NO_LOCATION, - allow_full_unsigned: bool = False, ) -> None: super().__init__(identifier, size, location) @@ -194,7 +193,7 @@ def simplify(expression: expr.Expr) -> expr.Expr: ) # Eng/RecordFlux/RecordFlux#1077 - # size of integers is limited to 63bits + # size of integers is limited to 63 bits if int(size_num) > const.MAX_SCALAR_SIZE: self.error.push( @@ -205,29 +204,6 @@ def simplify(expression: expr.Expr) -> expr.Expr: ), ) - if ( - not allow_full_unsigned - and int(first_num) == 0 - and int(last_num) == 2 ** int(size_num) - 1 - ): - assert self.location is not None - self.error.push( - ErrorEntry( - "unsigned integer syntax preferable", - Severity.ERROR, - self.location, - annotations=( - [ - Annotation( - f'use "type {self.name} is unsigned {int(size_num)}" instead', - Severity.HELP, - self.location, - ), - ] - ), - ), - ) - self.error.propagate() self._first_expr = first @@ -272,6 +248,40 @@ def last(self) -> expr.Number: def last_expr(self) -> expr.Expr: return self._last_expr + def check_style( + self, + error: RecordFluxError, + style_checks: dict[Path, frozenset[const.StyleCheck]], + ) -> None: + if not self.location or not self.location.source: + return + + checks = style_checks.get(self.location.source) + if not checks or const.StyleCheck.INTEGER_SYNTAX not in checks: + return + + if int(self.first) == 0 and int(self.last) == 2 ** int(self.size) - 1: + self.error.push( + ErrorEntry( + f'"{self.name}" covers the entire range of an unsigned integer type' + f" [style:{const.StyleCheck.INTEGER_SYNTAX.value}]", + Severity.ERROR, + self.location, + annotations=( + [ + Annotation( + f'use "type {self.name} is unsigned {int(self.size)}" instead', + Severity.HELP, + self.location, + ), + ] + ), + generate_default_annotation=False, + ), + ) + + error.extend(self.error.entries) + def constraints( self, name: str, @@ -343,12 +353,18 @@ def __init__( ), size, location=location, - allow_full_unsigned=True, ) def __str__(self) -> str: return f"type {self.name} is unsigned {self.size_expr}" + def check_style( + self, + error: RecordFluxError, + style_checks: dict[Path, frozenset[const.StyleCheck]], + ) -> None: + pass + class Enumeration(Scalar): def __init__( # noqa: PLR0912 @@ -720,14 +736,7 @@ def checked( skip_verification: bool = False, # noqa: ARG002 workers: int = 1, # noqa: ARG002 ) -> Integer: - return Integer( - self.identifier, - self.first, - self.last, - self.size, - location=self.location, - allow_full_unsigned=False, - ) + return Integer(self.identifier, self.first, self.last, self.size, self.location) @dataclass diff --git a/rflx/specification/parser.py b/rflx/specification/parser.py index 604e86930..1904f7d3a 100644 --- a/rflx/specification/parser.py +++ b/rflx/specification/parser.py @@ -6,7 +6,7 @@ from dataclasses import dataclass from pathlib import Path -from rflx import expr, lang, model, ty +from rflx import const, expr, lang, model, ty from rflx.common import STDIN, unique from rflx.const import RESERVED_WORDS from rflx.error import fail @@ -65,7 +65,7 @@ def diagnostics_to_error( error: RecordFluxError, filename: Path, ) -> bool: - """Append langkit diagnostics to RecordFlux error. Return True if error occured.""" + """Append langkit diagnostics to RecordFlux error. Return True if error occurred.""" if len(diagnostics) == 0: return False @@ -1758,12 +1758,14 @@ class SpecificationFile: filename: Path spec: lang.Specification context_clauses: list[ContextClause] + model_style_checks: frozenset[const.StyleCheck] @staticmethod def create( error: RecordFluxError, spec: lang.Specification, filename: Path, + model_style_checks: frozenset[const.StyleCheck], ) -> SpecificationFile: check_naming(error, spec.f_package_declaration, filename) @@ -1777,6 +1779,7 @@ def create( ) for context in spec.f_context_clause ], + model_style_checks, ) @property @@ -1842,7 +1845,13 @@ def parse_string( if not diagnostics_to_error(unit.diagnostics, error, filename): assert isinstance(unit.root, lang.Specification) - spec = SpecificationFile.create(error, unit.root, filename) + + basic_style_checks, model_style_checks = style.determine_enabled_checks( + error, + string, + filename, + ) + spec = SpecificationFile.create(error, unit.root, filename, model_style_checks) _check_for_duplicate_specifications(error, [*self._specifications.values(), spec]) self._specifications[spec.package] = spec @@ -1851,7 +1860,7 @@ def parse_string( self._specifications = _sort_specs_topologically(self._specifications) - error.extend(style.check_string(string, filename).entries) + style.check_string(error, string, basic_style_checks, filename) error.propagate() @@ -1862,10 +1871,12 @@ def create_unchecked_model(self) -> model.UncheckedModel: model.UNCHECKED_OPAQUE, ] + style_checks: dict[Path, frozenset[const.StyleCheck]] = {} for spec_node in self._specifications.values(): self._evaluate_specification(error, declarations, spec_node.spec, spec_node.filename) + style_checks[spec_node.filename] = spec_node.model_style_checks - return model.UncheckedModel(declarations, error) + return model.UncheckedModel(declarations, style_checks, error) def create_model(self) -> model.Model: unchecked_model = self.create_unchecked_model() @@ -1895,7 +1906,12 @@ def _parse_file(self, error: RecordFluxError, filename: Path) -> SpecificationFi source_code.register(filename, source_code_str) unit = lang.AnalysisContext().get_from_buffer(str(filename), source_code_str) - error.extend(style.check(filename).entries) + basic_style_checks, model_style_checks = style.determine_enabled_checks( + error, + source_code_str, + filename, + ) + error.extend(style.check(filename, basic_style_checks).entries) if diagnostics_to_error(unit.diagnostics, error, filename): return None @@ -1904,7 +1920,7 @@ def _parse_file(self, error: RecordFluxError, filename: Path) -> SpecificationFi self._integration.load_integration_file(filename, error) - return SpecificationFile.create(error, unit.root, filename) + return SpecificationFile.create(error, unit.root, filename, model_style_checks) def _parse_withed_files( self, diff --git a/rflx/specification/style.py b/rflx/specification/style.py index 234a6c080..4532ef500 100644 --- a/rflx/specification/style.py +++ b/rflx/specification/style.py @@ -1,9 +1,9 @@ from __future__ import annotations import re -from enum import Enum from pathlib import Path +from rflx.const import BASIC_STYLE_CHECKS, MODEL_STYLE_CHECKS, StyleCheck from rflx.rapidflux import ErrorEntry, Location, RecordFluxError, Severity INCORRECT_LINE_TERMINATORS = "\r" @@ -27,50 +27,98 @@ ] -class Check(Enum): - ALL = "all" - BLANK_LINES = "blank-lines" - CHARACTERS = "characters" - INDENTATION = "indentation" - LINE_LENGTH = "line-length" - TOKEN_SPACING = "token-spacing" - TRAILING_SPACES = "trailing-spaces" +def check(spec_file: Path, enabled_checks: frozenset[StyleCheck]) -> RecordFluxError: + """ + Perform basic style checks on the given specification file. - -def check(spec_file: Path) -> RecordFluxError: - with spec_file.open(encoding="utf-8", newline="") as f: - return check_string(f.read(), spec_file) + The function reads the specification file and performs style checks that can + be made directly on its string representation (basic checks). The exact + subset of checks to be performed must be passed via the `enabled_checks` + parameter. + """ + error = RecordFluxError() + # Read the file without any newline normalizations so that their conformance + # to the style rules can be checked. + with spec_file.open(newline="") as f: + source_code_str = f.read() + check_string(error, source_code_str, enabled_checks, spec_file) + return error -def check_string(specification: str, spec_file: Path = Path("")) -> RecordFluxError: - error = RecordFluxError() +def check_string( + error: RecordFluxError, + specification: str, + enabled_checks: frozenset[StyleCheck], + spec_file: Path = Path(""), +) -> None: + """ + Perform basic style checks on the given specification string. + The function performs style checks that can be made directly on the string + representation of the specification (basic checks). The exact subset of + checks to be performed must be passed via the `enabled_checks` parameter. + """ if specification in ("", "\n"): - return error - - lines = specification.split("\n") - enabled_checks = _determine_enabled_checks(error, lines[0], spec_file) + return if not enabled_checks: - return error + return - blank_lines = 0 + lines = specification.split("\n") + blank_lines = 0 for i, l in enumerate(lines, start=1): - if Check.BLANK_LINES in enabled_checks: + if StyleCheck.BLANK_LINES in enabled_checks: blank_lines = _check_blank_lines(error, l, i, spec_file, blank_lines, len(lines)) - if Check.CHARACTERS in enabled_checks: + if StyleCheck.CHARACTERS in enabled_checks: _check_characters(error, l, i, spec_file) - if Check.INDENTATION in enabled_checks: + if StyleCheck.INDENTATION in enabled_checks: _check_indentation(error, l, i, spec_file) - if Check.LINE_LENGTH in enabled_checks: + if StyleCheck.LINE_LENGTH in enabled_checks: _check_line_length(error, l, i, spec_file) - if Check.TOKEN_SPACING in enabled_checks: + if StyleCheck.TOKEN_SPACING in enabled_checks: _check_token_spacing(error, l, i, spec_file) - if Check.TRAILING_SPACES in enabled_checks: + if StyleCheck.TRAILING_SPACES in enabled_checks: _check_trailing_spaces(error, l, i, spec_file) - return error + +def determine_enabled_checks( + error: RecordFluxError, + specification: str, + spec_file: Path, +) -> tuple[frozenset[StyleCheck], frozenset[StyleCheck]]: + """ + Determine the enabled checks in a specification header. + + If the given specification contains a valid style check configuration, then + determine the enabled style checks. Otherwise, it is assumed that all checks + are enabled. + + The enabled checks are returned as a tuple where the first set of checks + (basic checks) can be performed directly on the input string and the second + set (model checks) must be performed at a model level. + """ + + header = specification.split("\n", 1)[0] + + all_checks = {c.value for c in StyleCheck.__members__.values()} + disabled_checks = set() + + m = re.match(r"^\s*--\s*style\s*:\s*disable\s*=\s*([^.]*)$", header) + if m: + disabled_checks = {c.strip() for c in m.group(1).split(",")} + for c in disabled_checks - all_checks: + _append(error, f'invalid check "{c}"', 1, 1, spec_file) + if StyleCheck.ALL.value in disabled_checks: + return frozenset(), frozenset() + else: + disabled_checks = set() + + enabled_checks = {StyleCheck(c) for c in all_checks - disabled_checks} + + return frozenset(enabled_checks - MODEL_STYLE_CHECKS), frozenset( + enabled_checks - BASIC_STYLE_CHECKS, + ) def _append( @@ -79,35 +127,17 @@ def _append( row: int, col: int, spec_file: Path, - check_type: Check | None = None, + check_type: StyleCheck | None = None, ) -> None: error.push( ErrorEntry( - message + (f" [{check_type.value}]" if check_type else ""), + message + (f" [style:{check_type.value}]" if check_type else ""), Severity.ERROR, Location((row, col), spec_file), ), ) -def _determine_enabled_checks(error: RecordFluxError, line: str, spec_file: Path) -> set[Check]: - checks = {c.value for c in Check.__members__.values()} - disabled_checks = set() - - m = re.match(r"^\s*--\s*style\s*:\s*disable\s*=\s*([^.]*)$", line) - if m: - disabled_checks = {c.strip() for c in m.group(1).split(",")} - for c in disabled_checks - checks: - _append(error, f'invalid check "{c}"', 1, 1, spec_file) - else: - return set(Check.__members__.values()) - - if Check.ALL.value in disabled_checks: - return set() - - return {Check(c) for c in checks - disabled_checks} - - def _check_blank_lines( error: RecordFluxError, line: str, @@ -118,13 +148,13 @@ def _check_blank_lines( ) -> int: if not line: if row == 1: - _append(error, "leading blank line", row, 1, spec_file, Check.BLANK_LINES) + _append(error, "leading blank line", row, 1, spec_file, StyleCheck.BLANK_LINES) if blank_lines > 0 and row == row_count: - _append(error, "trailing blank line", row - 1, 1, spec_file, Check.BLANK_LINES) + _append(error, "trailing blank line", row - 1, 1, spec_file, StyleCheck.BLANK_LINES) blank_lines += 1 else: if blank_lines > 1: - _append(error, "multiple blank lines", row - 1, 1, spec_file, Check.BLANK_LINES) + _append(error, "multiple blank lines", row - 1, 1, spec_file, StyleCheck.BLANK_LINES) blank_lines = 0 return blank_lines @@ -134,10 +164,24 @@ def _check_characters(error: RecordFluxError, line: str, row: int, spec_file: Pa for j, c in enumerate(line, start=1): if c == INCORRECT_LINE_TERMINATORS: s = repr(c).replace("'", '"') - _append(error, f"incorrect line terminator {s}", row, j, spec_file, Check.CHARACTERS) + _append( + error, + f"incorrect line terminator {s}", + row, + j, + spec_file, + StyleCheck.CHARACTERS, + ) if c in ILLEGAL_WHITESPACE_CHARACTERS: s = repr(c).replace("'", '"') - _append(error, f"illegal whitespace character {s}", row, j, spec_file, Check.CHARACTERS) + _append( + error, + f"illegal whitespace character {s}", + row, + j, + spec_file, + StyleCheck.CHARACTERS, + ) def _check_indentation(error: RecordFluxError, line: str, row: int, spec_file: Path) -> None: @@ -158,7 +202,7 @@ def _check_indentation(error: RecordFluxError, line: str, row: int, spec_file: P row, match.end() + 1, spec_file, - Check.INDENTATION, + StyleCheck.INDENTATION, ) @@ -206,7 +250,7 @@ def _check_token_spacing( # noqa: PLR0912 row, match.start() + 1, spec_file, - Check.TOKEN_SPACING, + StyleCheck.TOKEN_SPACING, ) else: if match.start() > 1 and line[match.start() - 1] == " ": @@ -216,7 +260,7 @@ def _check_token_spacing( # noqa: PLR0912 row, match.start() + 1, spec_file, - Check.TOKEN_SPACING, + StyleCheck.TOKEN_SPACING, ) if space_after: if match.end() < len(line) and line[match.end()] not in " ;\n": @@ -226,7 +270,7 @@ def _check_token_spacing( # noqa: PLR0912 row, match.end() + 1, spec_file, - Check.TOKEN_SPACING, + StyleCheck.TOKEN_SPACING, ) else: if match.end() < len(line) and line[match.end()] == " ": @@ -236,15 +280,22 @@ def _check_token_spacing( # noqa: PLR0912 row, match.end() + 2, spec_file, - Check.TOKEN_SPACING, + StyleCheck.TOKEN_SPACING, ) def _check_trailing_spaces(error: RecordFluxError, line: str, row: int, spec_file: Path) -> None: if line.endswith(" "): - _append(error, "trailing whitespace", row, len(line), spec_file, Check.TRAILING_SPACES) + _append(error, "trailing whitespace", row, len(line), spec_file, StyleCheck.TRAILING_SPACES) def _check_line_length(error: RecordFluxError, line: str, row: int, spec_file: Path) -> None: if len(line) > 120: - _append(error, f"line too long ({len(line)}/120)", row, 121, spec_file, Check.LINE_LENGTH) + _append( + error, + f"line too long ({len(line)}/120)", + row, + 121, + spec_file, + StyleCheck.LINE_LENGTH, + ) diff --git a/tests/end_to_end/cli_test.py b/tests/end_to_end/cli_test.py index 4d511a3c5..d636f3727 100644 --- a/tests/end_to_end/cli_test.py +++ b/tests/end_to_end/cli_test.py @@ -348,3 +348,65 @@ def test_unexpected_exception( capfd.readouterr().err, re.DOTALL, ) + + +@pytest.mark.parametrize( + ("spec"), + [ + ("package Test is\r\nend Test;"), + ], +) +def test_style_newlines_error(spec: str, tmp_path: Path) -> None: + spec_file = tmp_path / "test.rflx" + with spec_file.open("w", newline="") as f: + f.write(spec) + p = subprocess.run( + ["rflx", "--no-caching", "check", spec_file], + capture_output=True, + check=False, + ) + assert p.returncode == 1 + assert p.stdout.decode("utf-8") == "" + assert p.stderr.decode("utf-8") == textwrap.dedent( + f"""\ + info: Parsing {spec_file} + info: Processing Test + info: Verifying __BUILTINS__::Boolean + info: Verifying __INTERNAL__::Opaque + error: incorrect line terminator "\\r" [style:characters] + --> {spec_file}:1:16 + | + 1 | package Test is + | ^ + | + """, + ) + + +@pytest.mark.parametrize( + ("spec"), + [ + ("package Test is\nend Test;"), + ("-- style: disable = characters\npackage Test is\r\nend Test;"), + ], +) +def test_style_newlines_no_error(spec: str, tmp_path: Path) -> None: + spec_file = tmp_path / "test.rflx" + with spec_file.open("w", newline="") as f: + f.write(spec) + spec_file.write_text(spec) + p = subprocess.run( + ["rflx", "--no-caching", "check", spec_file], + capture_output=True, + check=False, + ) + assert p.returncode == 0 + assert p.stdout.decode("utf-8") == "" + assert p.stderr.decode("utf-8") == textwrap.dedent( + f"""\ + info: Parsing {spec_file} + info: Processing Test + info: Verifying __BUILTINS__::Boolean + info: Verifying __INTERNAL__::Opaque + """, + ) diff --git a/tests/integration/cli_test.py b/tests/integration/cli_test.py index e879341aa..9b8a6348e 100644 --- a/tests/integration/cli_test.py +++ b/tests/integration/cli_test.py @@ -7,7 +7,7 @@ from rflx.rapidflux import RecordFluxError -def test_parse_no_subsequent_errors_caused_by_style_errors(tmp_path: Path) -> None: +def test_parse_no_subsequent_errors_caused_by_basic_style_errors(tmp_path: Path) -> None: a = tmp_path / "a.rflx" a.write_text( textwrap.dedent( @@ -55,8 +55,106 @@ def test_parse_no_subsequent_errors_caused_by_style_errors(tmp_path: Path) -> No match=( r"^" rf"{a}:12:1: error: unexpected keyword indentation \(expected 3, 12 or 15\)" - r" \[indentation\]" + r" \[style:indentation\]" r"$" ), ): cli.parse([a], no_caching=True, no_verification=False) + + +def test_parse_no_subsequent_errors_caused_by_model_style_errors(tmp_path: Path) -> None: + p = tmp_path / "p.rflx" + p.write_text( + textwrap.dedent( + """\ + package P is + type I is range 0 .. 2 ** 4 - 1 with Size => 4; + type M is + message + F : I; + G : I; + end message; + end P; + """, + ), + ) + + with pytest.raises( + RecordFluxError, + match=( + r"^" + rf'{p}:2:9: error: "I" covers the entire range of an unsigned integer type' + r" \[style:integer-syntax\]\n" + rf'{p}:2:9: help: use "type I is unsigned 4" instead' + r"$" + ), + ): + cli.parse([p], no_caching=True, no_verification=False) + + +@pytest.mark.parametrize( + ("spec"), + [ + """\ + -- no style header + package P is + type I is range 0 .. 2 ** 4 - 1 with Size => 4; + end P; + """, + """\ + -- style: disable = line-length, blank-lines + package P is + type I is range 0 .. 15 with Size => 4; + end P; + """, + ], +) +def test_integer_style_error(spec: str, tmp_path: Path) -> None: + p = tmp_path / "p.rflx" + p.write_text(textwrap.dedent(spec)) + + with pytest.raises( + RecordFluxError, + match=( + r"^" + rf'{p}:3:9: error: "I" covers the entire range of an unsigned integer type' + r" \[style:integer-syntax\]\n" + rf'{p}:3:9: help: use "type I is unsigned 4" instead' + r"$" + ), + ): + cli.parse([p], no_caching=True, no_verification=False) + + +@pytest.mark.parametrize( + ("spec"), + [ + """\ + package P is + type I is range 1 .. 15 with Size => 4; + end P; + """, + """\ + -- style: disable = line-length, integer-syntax, blank-lines + package P is + type I is range 0 .. 15 with Size => 4; + end P; + """, + """\ + -- style: disable = all + package P is + type I is range 0 .. 15 with Size => 4; + end P; + """, + """\ + package P is + type U is unsigned 4; + end P; + """, + ], +) +def test_integer_style_no_error(spec: str, tmp_path: Path) -> None: + p = tmp_path / "p.rflx" + p.write_text(textwrap.dedent(spec)) + + cli.parse([p], no_caching=True, no_verification=False) diff --git a/tests/tools/check_doc_test.py b/tests/tools/check_doc_test.py index c84054f1f..806425411 100644 --- a/tests/tools/check_doc_test.py +++ b/tests/tools/check_doc_test.py @@ -215,7 +215,7 @@ def test_invalid_rflx_rule_style() -> None: CheckDocError, match=( r"^:6: error in code block\n" - r':1:8: error: missing space after "\*\*" \[token-spacing\]$' + r':1:8: error: missing space after "\*\*" \[style:token-spacing\]$' ), ): check_file( @@ -239,7 +239,7 @@ def test_invalid_rflx_spec_style() -> None: match=( r"^:6: error in code block\n" r":3:5: error: unexpected keyword indentation \(expected 3 or 6\) " - r"\[indentation\]$" + r"\[style:indentation\]$" ), ): check_file( diff --git a/tests/unit/ls/lexer_test.py b/tests/unit/ls/lexer_test.py index 8c1490682..842c312c3 100644 --- a/tests/unit/ls/lexer_test.py +++ b/tests/unit/ls/lexer_test.py @@ -20,7 +20,7 @@ def model() -> LSModel: def test_tokenization_with_empty_model() -> None: - lexer = LSLexer(LSModel(UncheckedModel([], RecordFluxError()))) + lexer = LSLexer(LSModel(UncheckedModel([], {}, RecordFluxError()))) lexer.tokenize((DATA_DIR / "message.rflx").read_text()) lexer.tokenize((DATA_DIR / "state_machine.rflx").read_text()) tokens = lexer.tokens diff --git a/tests/unit/ls/server_test.py b/tests/unit/ls/server_test.py index 9bea9c58f..73a5231c3 100644 --- a/tests/unit/ls/server_test.py +++ b/tests/unit/ls/server_test.py @@ -249,7 +249,12 @@ def test_verify(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: document = tmp_path / "test.rflx" document.write_text( - "package Test is type T is unsigned 64; end Test;", + """\ + package Test is + type T1 is unsigned 64; + type T2 is range 0 .. 255 with Size => 8; + end Test; + """, ) document_uri = document.absolute().as_uri() @@ -268,8 +273,14 @@ def test_verify(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: document.absolute().as_uri(), [ Diagnostic( - Range(Position(0, 35), Position(0, 37)), - 'last of "T" exceeds limit (2**63 - 1)', + Range(Position(1, 23), Position(1, 25)), + 'last of "T1" exceeds limit (2**63 - 1)', + DiagnosticSeverity.Error, + ), + Diagnostic( + Range(Position(2, 8), Position(2, 43)), + '"T2" covers the entire range of an unsigned integer type' + " [style:integer-syntax]", DiagnosticSeverity.Error, ), ], diff --git a/tests/unit/model/message_test.py b/tests/unit/model/message_test.py index 312cd7517..c2aad1aac 100644 --- a/tests/unit/model/message_test.py +++ b/tests/unit/model/message_test.py @@ -1192,7 +1192,11 @@ def test_invalid_relation_to_aggregate() -> None: @pytest.mark.parametrize("lower", [Number(0), Number(1)]) def test_invalid_element_in_relation_to_aggregate(lower: Number) -> None: - integer = Integer("P::Integer", lower, Number(255), Number(8), allow_full_unsigned=True) + integer = ( + UnsignedInteger("P::Integer", Number(8)) + if lower == Number(0) + else Integer("P::Integer", lower, Number(255), Number(8)) + ) structure = [ Link(INITIAL, Field(ID("F1", location=Location((1, 2))))), Link( diff --git a/tests/unit/model/model_test.py b/tests/unit/model/model_test.py index d555eedf0..279f275f5 100644 --- a/tests/unit/model/model_test.py +++ b/tests/unit/model/model_test.py @@ -571,7 +571,9 @@ def test_unchecked_model_checked( declarations = [d() for d in expected] - assert UncheckedModel(unchecked, RecordFluxError()).checked(cache=cache) == Model(declarations) + assert UncheckedModel(unchecked, {}, RecordFluxError()).checked(cache=cache) == Model( + declarations, + ) messages = [d for d in declarations if isinstance(d, Message)] if messages: @@ -731,6 +733,6 @@ def test_unchecked_model_checked_error( cache = Cache(tmp_path / "test.json") with pytest.raises(RecordFluxError, match=expected): - UncheckedModel(unchecked, RecordFluxError()).checked(cache=cache) + UncheckedModel(unchecked, {}, RecordFluxError()).checked(cache=cache) assert list(cache._verified) == expect_cached # noqa: SLF001 diff --git a/tests/unit/model/type_decl_test.py b/tests/unit/model/type_decl_test.py index 3043c2c54..1cab441d6 100644 --- a/tests/unit/model/type_decl_test.py +++ b/tests/unit/model/type_decl_test.py @@ -1,8 +1,11 @@ +from __future__ import annotations + from collections.abc import Callable +from pathlib import Path import pytest -from rflx import ty +from rflx import const, ty from rflx.expr import Add, Aggregate, Equal, Mul, Number, Pow, Size, Sub, Variable from rflx.identifier import ID from rflx.model import ( @@ -322,21 +325,65 @@ def test_integer_invalid_out_of_bounds() -> None: ) -def test_integer_can_be_unsigned() -> None: +def test_integer_style_check() -> None: + source1 = Path("file1.rflx") + source2 = Path("file2.rflx") + + style_checks: dict[Path, frozenset[const.StyleCheck]] = {} + style_checks[source1] = frozenset() + style_checks[source2] = frozenset([const.StyleCheck.INTEGER_SYNTAX]) + + i = Integer( + "P::I", + Number(0), + Number(63), + Number(6), + Location((10, 9), source1), + ) + + error = RecordFluxError() + + i.check_style(error, style_checks) # Should pass + error.propagate() + + i.location = Location((10, 9), source2) + + i.check_style(error, style_checks) with pytest.raises( RecordFluxError, match=( - r"^:10:5: error: unsigned integer syntax preferable\n" - r':10:5: help: use "type T is unsigned 6" instead$' + r"^" + rf'{source2}:10:9: error: "I" covers the entire range of an unsigned integer type' + r" \[style:integer-syntax\]\n" + rf'{source2}:10:9: help: use "type I is unsigned 6" instead' + r"$" ), ): - Integer( - "P::T", - Number(0, location=Location((10, 3))), - Number(63, location=Location((10, 4))), - Number(6, location=Location((10, 6))), - Location((10, 5)), - ) + error.propagate() + + +def test_unsigned_style_check() -> None: + source1 = Path("file1.rflx") + source2 = Path("file2.rflx") + + style_checks: dict[Path, frozenset[const.StyleCheck]] = {} + style_checks[source1] = frozenset() + style_checks[source2] = frozenset([const.StyleCheck.INTEGER_SYNTAX]) + + u = UnsignedInteger( + "P::U", + Number(3, location=Location((5, 24))), + Location((5, 9), source1), + ) + + error = RecordFluxError() + + u.check_style(error, style_checks) # Should pass + error.propagate() + + u.location = Location((5, 9), source2) + u.check_style(error, style_checks) # Should pass + error.propagate() def test_enumeration_size() -> None: diff --git a/tests/unit/specification/parser_test.py b/tests/unit/specification/parser_test.py index 837f5f898..9c81b4e82 100644 --- a/tests/unit/specification/parser_test.py +++ b/tests/unit/specification/parser_test.py @@ -326,7 +326,7 @@ def test_parse_string_error() -> None: 'a/b.rflx:1:9: help: rename to "B"\n' 'a/b.rflx:3:5: help: rename to "B"\n' "a/b.rflx:2:1: error: unexpected keyword indentation (expected 3 or 6) " - "[indentation]", + "[style:indentation]", ), ) ), diff --git a/tests/unit/specification/style_test.py b/tests/unit/specification/style_test.py index 469a83436..1391fc9b1 100644 --- a/tests/unit/specification/style_test.py +++ b/tests/unit/specification/style_test.py @@ -1,14 +1,26 @@ +from __future__ import annotations + import re import textwrap from pathlib import Path import pytest +from rflx import const from rflx.rapidflux import RecordFluxError, source_code from rflx.specification import style from tests.utils import assert_stderr_regex +def write_spec_and_check(spec: str, spec_file: Path) -> RecordFluxError: + error = RecordFluxError() + with spec_file.open("w", newline="") as f: + f.write(spec) + basic_style_checks, _ = style.determine_enabled_checks(error, spec, spec_file) + error.extend(style.check(spec_file, basic_style_checks).entries) + return error + + @pytest.mark.parametrize( "spec", [ @@ -82,10 +94,10 @@ """, ], ) -def test_no_error(spec: str, tmp_path: Path) -> None: +def test_check_no_error(spec: str, tmp_path: Path) -> None: spec_file = tmp_path / "test.rflx" spec_file.write_text(spec) - style.check(spec_file).propagate() + style.check(spec_file, const.BASIC_STYLE_CHECKS).propagate() @pytest.mark.parametrize( @@ -93,73 +105,69 @@ def test_no_error(spec: str, tmp_path: Path) -> None: [ ( "\npackage Test is end Test;", - r"1:1: error: leading blank line \[blank-lines\]", + r"1:1: error: leading blank line \[style:blank-lines\]", ), ( "package Test is end Test;\n\n", - r"2:1: error: trailing blank line \[blank-lines\]", + r"2:1: error: trailing blank line \[style:blank-lines\]", ), ( "package Test is\n\n\nend Test;", - r"3:1: error: multiple blank lines \[blank-lines\]", + r"3:1: error: multiple blank lines \[style:blank-lines\]", ), ( """package Test is\tend Test;""", - r'1:16: error: illegal whitespace character "\\t" \[characters\]', + r'1:16: error: illegal whitespace character "\\t" \[style:characters\]', ), ( "package Test is\r\nend Test;", - r'1:16: error: incorrect line terminator "\\r" \[characters\]', + r'1:16: error: incorrect line terminator "\\r" \[style:characters\]', ), ( "package Test is end Test; ", - r"1:26: error: trailing whitespace \[trailing-spaces\]", + r"1:26: error: trailing whitespace \[style:trailing-spaces\]", ), ( "package Test is\n type T is range 0 .. 2 ** 16 with Size => 16;\nend Test;", - r"2:10: error: unexpected keyword indentation \(expected 3 or 6\) \[indentation\]", + r"2:10: error: unexpected keyword indentation \(expected 3 or 6\)" + r" \[style:indentation\]", ), ( "package Test is\n type T is mod 2* 128;\nend Test;", - r'2:19: error: missing space before "\*" \[token-spacing\]', + r'2:19: error: missing space before "\*" \[style:token-spacing\]', ), ( "package Test is end Test; --A test package", - r'1:29: error: missing space after "--" \[token-spacing\]', + r'1:29: error: missing space after "--" \[style:token-spacing\]', ), ( f"package Test is end Test; -- {'X' * 100}", - r"1:121: error: line too long \(129/120\) \[line-length\]", + r"1:121: error: line too long \(129/120\) \[style:line-length\]", ), ( "package Test is\n" " type E is unsigned 16;\n" " type S is sequence of Test ::E;\n" "end Test;", - r'3:31: error: space before "::" \[token-spacing\]', + r'3:31: error: space before "::" \[style:token-spacing\]', ), ( "package Test is\n" " type E is unsigned 16;\n" " type S is sequence of Test:: E;\n" "end Test;", - r'3:33: error: space after "::" \[token-spacing\]', - ), - ( - "-- style: disable = foo", - r'1:1: error: invalid check "foo"', + r'3:33: error: space after "::" \[style:token-spacing\]', ), ], ) -def test_error(tmp_path: Path, spec: str, error: str) -> None: +def test_check_error(tmp_path: Path, spec: str, error: str) -> None: spec_file = tmp_path / "test.rflx" - spec_file.write_text(spec) with pytest.raises(RecordFluxError, match=rf"^{spec_file}:{error}$"): - style.check(spec_file).propagate() + write_spec_and_check(spec, spec_file).propagate() @pytest.mark.parametrize( - ("spec", "disabled_checks"), + ("base_spec", "disabled_checks"), [ ( "package Test is end Test;\n\n", @@ -228,13 +236,90 @@ def test_error(tmp_path: Path, spec: str, error: str) -> None: ], ) def test_deactivation_of_checks_on_file_level( - spec: str, + base_spec: str, disabled_checks: str, tmp_path: Path, +) -> None: + spec = f"-- style: disable = {disabled_checks}\n\n{base_spec}" + spec_file = tmp_path / "test.rflx" + write_spec_and_check(spec, spec_file).propagate() + + +@pytest.mark.parametrize( + ("header", "expected_basic_style_checks", "expected_model_style_checks"), + [ + ( + "", + const.BASIC_STYLE_CHECKS, + const.MODEL_STYLE_CHECKS, + ), + ( + "-- style: disable = all\n", + set(), + set(), + ), + ( + "-- style: disable = indentation\n", + const.BASIC_STYLE_CHECKS - {const.StyleCheck("indentation")}, + {const.StyleCheck("integer-syntax")}, + ), + ( + "-- style: disable = integer-syntax\n", + const.BASIC_STYLE_CHECKS, + const.MODEL_STYLE_CHECKS - {const.StyleCheck("integer-syntax")}, + ), + ( + "-- style: disable = indentation,integer-syntax\n", + const.BASIC_STYLE_CHECKS - {const.StyleCheck("indentation")}, + const.MODEL_STYLE_CHECKS - {const.StyleCheck("integer-syntax")}, + ), + ( + "-- style: disable = integer-syntax,indentation\n", + const.BASIC_STYLE_CHECKS - {const.StyleCheck("indentation")}, + const.MODEL_STYLE_CHECKS - {const.StyleCheck("integer-syntax")}, + ), + ], +) +def test_header_no_error( + header: str, + expected_basic_style_checks: set[const.StyleCheck], + expected_model_style_checks: set[const.StyleCheck], + tmp_path: Path, +) -> None: + spec_file = tmp_path / "test.rflx" + spec = f"{header}\npackage Test is end Test;" + spec_file.write_text(spec) + error = RecordFluxError() + basic_style_checks, model_style_checks = style.determine_enabled_checks( + error, + spec.split("\n", 1)[0], + spec_file, + ) + assert basic_style_checks == expected_basic_style_checks + assert model_style_checks == expected_model_style_checks + + +@pytest.mark.parametrize( + ("header", "error_msg"), + [ + ( + "-- style: disable = foo", + r'1:1: error: invalid check "foo"', + ), + ], +) +def test_header_error( + header: str, + error_msg: str, + tmp_path: Path, ) -> None: spec_file = tmp_path / "test.rflx" - spec_file.write_text(f"-- style: disable = {disabled_checks}\n\n{spec}") - style.check(spec_file).propagate() + spec = f"{header}\npackage Test is end Test;" + spec_file.write_text(spec) + error = RecordFluxError() + _, _ = style.determine_enabled_checks(error, spec, spec_file) + with pytest.raises(RecordFluxError, match=rf"^{spec_file}:{error_msg}$"): + error.propagate() @pytest.mark.parametrize( @@ -253,7 +338,7 @@ def test_deactivation_of_checks_on_file_level( ), textwrap.dedent( """\ - error: unexpected keyword indentation (expected 3 or 6) [indentation] + error: unexpected keyword indentation (expected 3 or 6) [style:indentation] --> {}:2:1 | 2 | type M is @@ -275,7 +360,7 @@ def test_deactivation_of_checks_on_file_level( ), textwrap.dedent( """\ - error: unexpected keyword indentation (expected 3 or 6) [indentation] + error: unexpected keyword indentation (expected 3 or 6) [style:indentation] --> {}:2:5 | 2 | type M is @@ -296,5 +381,5 @@ def test_incorrect_indentation_message_display( spec_file.write_text(spec_code) source_code.register(spec_file, spec_file.read_text()) - style.check(spec_file).print_messages() + style.check(spec_file, const.BASIC_STYLE_CHECKS).print_messages() assert_stderr_regex(f"^{re.escape(expected_message.format(spec_file))}$", capfd) diff --git a/tools/check_doc.py b/tools/check_doc.py index d79ba8119..980e10055 100755 --- a/tools/check_doc.py +++ b/tools/check_doc.py @@ -24,6 +24,7 @@ from ruamel.yaml.parser import ParserError from rflx.common import STDIN +from rflx.const import BASIC_STYLE_CHECKS from rflx.lang import AnalysisContext, GrammarRule from rflx.rapidflux import RecordFluxError from rflx.specification import Parser, style @@ -365,7 +366,7 @@ def parse(data: str, rule: str) -> None: error = RecordFluxError() if diagnostics_to_error(unit.diagnostics, error, STDIN): error.propagate() - error.extend(style.check_string(data).entries) + style.check_string(error, data, BASIC_STYLE_CHECKS) error.propagate()