diff options
author | Daniel Baumann <daniel.baumann@progress-linux.org> | 2024-05-24 04:48:18 +0000 |
---|---|---|
committer | Daniel Baumann <daniel.baumann@progress-linux.org> | 2024-05-24 04:48:18 +0000 |
commit | 957b31fb2be1dfabdef378e5f41817ec45b12cf0 (patch) | |
tree | d80cb1204e4226f12deb0f95ae37248b16bc7ebc | |
parent | Adding debian version 0.1.33. (diff) | |
download | debputy-957b31fb2be1dfabdef378e5f41817ec45b12cf0.tar.xz debputy-957b31fb2be1dfabdef378e5f41817ec45b12cf0.zip |
Merging upstream version 0.1.34.
Signed-off-by: Daniel Baumann <daniel.baumann@progress-linux.org>
Diffstat (limited to '')
-rw-r--r-- | .gitlab-ci.yml | 7 | ||||
-rw-r--r-- | debputy.pod | 58 | ||||
-rw-r--r-- | src/debputy/commands/debputy_cmd/lint_and_lsp_cmds.py | 14 | ||||
-rw-r--r-- | src/debputy/deb_packaging_support.py | 8 | ||||
-rw-r--r-- | src/debputy/linting/lint_impl.py | 217 | ||||
-rw-r--r-- | src/debputy/linting/lint_report_junit.py | 75 | ||||
-rw-r--r-- | src/debputy/linting/lint_util.py | 345 | ||||
-rw-r--r-- | src/debputy/lsp/diagnostics.py | 2 | ||||
-rw-r--r-- | src/debputy/lsp/lsp_debian_changelog.py | 5 | ||||
-rw-r--r-- | src/debputy/lsp/lsp_debian_control.py | 91 | ||||
-rw-r--r-- | src/debputy/lsp/lsp_debian_control_reference_data.py | 234 | ||||
-rw-r--r-- | src/debputy/lsp/lsp_debian_copyright.py | 18 | ||||
-rw-r--r-- | src/debputy/lsp/lsp_debian_tests_control.py | 18 | ||||
-rw-r--r-- | src/debputy/lsp/style-preferences.yaml | 4 | ||||
-rw-r--r-- | tests/lint_tests/test_lint_changelog.py | 39 | ||||
-rw-r--r-- | tests/lint_tests/test_lint_dctrl.py | 160 | ||||
-rw-r--r-- | tests/lint_tests/test_lint_dtctrl.py | 74 | ||||
-rw-r--r-- | tests/test_style.py | 4 |
18 files changed, 1080 insertions, 293 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index c0c5ccf..d6cf11d 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -214,8 +214,13 @@ debputy-lint: stage: ci-test image: debian:sid-slim script: - - apt-get update -qq && apt-get -qq build-dep --no-install-recommends --yes . + - apt-get update -qq && apt-get -qq build-dep --no-install-recommends --yes . && apt-get -qq install --yes python3-levenshtein python3-junit.xml + - PERL5LIB=lib ./debputy.sh lint --lint-report-format=junit4-xml --report-output debputy-lint-report.xml + # Mostly just for the validation that --spellcheck does not crash - PERL5LIB=lib ./debputy.sh lint --spellcheck except: variables: - $CI_COMMIT_TAG != null && $SALSA_CI_ENABLE_PIPELINE_ON_TAGS !~ /^(1|yes|true)$/ + artifacts: + reports: + junit: debputy-lint-report.xml diff --git a/debputy.pod b/debputy.pod index 3687398..f27c15f 100644 --- a/debputy.pod +++ b/debputy.pod @@ -248,6 +248,30 @@ providing the results. If you rely on the exit code, you are recommended to explicitly pass the relevant variant of the flag even if the current default matches your wishes. +=item B<--lint-report-format> I<term|junit-xml> + +Choose the output format of the resulting report. The B<term> report is a terminal output report. +The B<junit-xml> writes the output to an XML in a JUnit 4 format (should be compatible with +the xunit2 family of JUnit4 style input). This is useful for GitLab CI pipelines to get the +results imported via GitLab's B<junit> CI pipeline feature. + +=item B<--report-output> I<DEST> + +For reports that generate file system artifacts, choose where the output should be placed. + +Ignored with a warning for formats that do not generate file system outputs such as the +B<term> report format. + +=item B<--warn-about-check-manifest>, B<--no-warn-about-check-manifest> + +Whether B<debputy lint> should remind you that it has short comings in regards to validating +the B<debian/debputy.manifest> file. The warning will only appear if the manifest file exists. +That is, the options have no effect either way when the file is not present. + +The B<--no-warn-about-check-manifest> is mostly used for generic pipelines that separately +call B<debputy check-manifest> to ensure that the manifest is also validated or for cases +where the limitation is known and accepted. + =back A short comparison of B<debputy lint> vs. other tools: @@ -324,36 +348,12 @@ known variations such as B<debian-control> (as used by B<eglot> from B<emacs>) a B<debcontrol> (as used by B<vim>). See B<debputy lsp features> for a list of known language IDs along with their aliases. -When properly set up, the language server will offer a variety of features such as: - -=over 4 - -=item - - -Completion suggestion such as field names or values in deb822 based files. - -=item - - -Hover documentation for fields in deb822-based files. - -=item - - -Diagnostics ("linting"). In many cases, B<debputy> will also provide quickfixes for the issues. - -This feature is also (partly) available via the B<debputy lint> command. The primary limitation -in B<debputy lint> is that you cannot interactively choose which quickfix to apply. - -=item - - -On save actions (currently only "prune trailing whitespace"). - -=item - - -Folding ranges (multi-line comments). - -=back +When properly set up, the language server will offer a variety of features such as completion +suggestions, hover documentation, "as you type" diagnostics, quickfixes, etc. Please see +B<debputy lsp features> for the full list of features per format. That command will also +help identify mandatory and optional dependencies for the B<debputy lsp server> command. -Note these features are subject to the editor supporting them, correct language IDs being +Note many of the features are subject to the editor supporting them, correct language IDs being passed to B<debputy>, etc. Options for this subcommand diff --git a/src/debputy/commands/debputy_cmd/lint_and_lsp_cmds.py b/src/debputy/commands/debputy_cmd/lint_and_lsp_cmds.py index 8189535..ee64a84 100644 --- a/src/debputy/commands/debputy_cmd/lint_and_lsp_cmds.py +++ b/src/debputy/commands/debputy_cmd/lint_and_lsp_cmds.py @@ -269,6 +269,20 @@ def lsp_describe_features(context: CommandContext) -> None: help='Enable or disable the "linter" convention of exiting with an error if severe issues were found', ), add_arg( + "--lint-report-format", + dest="lint_report_format", + default="term", + choices=["term", "junit4-xml"], + help="The report output format", + ), + add_arg( + "--report-output", + dest="report_output", + default=None, + action="store", + help="Where to place the report (for report formats that generate files/directory reports)", + ), + add_arg( "--warn-about-check-manifest", dest="warn_about_check_manifest", default=True, diff --git a/src/debputy/deb_packaging_support.py b/src/debputy/deb_packaging_support.py index 8ab0484..c88841f 100644 --- a/src/debputy/deb_packaging_support.py +++ b/src/debputy/deb_packaging_support.py @@ -8,6 +8,7 @@ import itertools import operator import os import re +import shutil import subprocess import tempfile import textwrap @@ -698,12 +699,17 @@ def _attach_debug(objcopy: str, elf_binary: VirtualPath, dbgsym: FSPath) -> None ) +@functools.lru_cache() +def _has_tool(tool: str) -> bool: + return shutil.which(tool) is not None + + def _run_dwz( dctrl: BinaryPackage, dbgsym_fs_root: FSPath, unstripped_elf_info: List[_ElfInfo], ) -> None: - if not unstripped_elf_info or dctrl.is_udeb: + if not unstripped_elf_info or dctrl.is_udeb or not _has_tool("dwz"): return dwz_cmd = ["dwz"] dwz_ma_dir_name = f"usr/lib/debug/.dwz/{dctrl.deb_multiarch}" diff --git a/src/debputy/linting/lint_impl.py b/src/debputy/linting/lint_impl.py index 6ccf03d..63f4e77 100644 --- a/src/debputy/linting/lint_impl.py +++ b/src/debputy/linting/lint_impl.py @@ -6,15 +6,28 @@ import sys import textwrap from typing import Optional, List, Union, NoReturn, Mapping +from lsprotocol.types import ( + CodeAction, + Command, + CodeActionParams, + CodeActionContext, + TextDocumentIdentifier, + TextEdit, + Position, + DiagnosticSeverity, + Diagnostic, +) + from debputy.commands.debputy_cmd.context import CommandContext from debputy.commands.debputy_cmd.output import _output_styling, OutputStylingBase from debputy.filesystem_scan import FSROOverlay from debputy.linting.lint_util import ( - report_diagnostic, LinterImpl, LintReport, LintStateImpl, FormatterImpl, + TermLintReport, + LintDiagnosticResultState, ) from debputy.lsp.lsp_debian_changelog import _lint_debian_changelog from debputy.lsp.lsp_debian_control import ( @@ -52,17 +65,6 @@ from debputy.plugin.api.feature_set import PluginProvidedFeatureSet from debputy.util import _warn, _error, _info from debputy.yaml import MANIFEST_YAML, YAMLError from debputy.yaml.compat import CommentedMap -from lsprotocol.types import ( - CodeAction, - Command, - CodeActionParams, - CodeActionContext, - TextDocumentIdentifier, - TextEdit, - Position, - DiagnosticSeverity, - Diagnostic, -) LINTER_FORMATS = { "debian/changelog": _lint_debian_changelog, @@ -156,13 +158,39 @@ def gather_lint_info(context: CommandContext) -> LintContext: return lint_context +def initialize_lint_report(context: CommandContext) -> LintReport: + lint_report_format = context.parsed_args.lint_report_format + report_output = context.parsed_args.report_output + + if lint_report_format == "term": + fo = _output_styling(context.parsed_args, sys.stdout) + if report_output is not None: + _warn("--report-output is redundant for the `term` report") + return TermLintReport(fo) + if lint_report_format == "junit4-xml": + try: + import junit_xml + except ImportError: + _error( + "The `junit4-xml` report format requires `python3-junit.xml` to be installed" + ) + + from debputy.linting.lint_report_junit import JunitLintReport + + if report_output is None: + report_output = "debputy-lint-junit.xml" + + return JunitLintReport(report_output) + + raise AssertionError(f"Missing case for lint_report_format: {lint_report_format}") + + def perform_linting(context: CommandContext) -> None: parsed_args = context.parsed_args if not parsed_args.spellcheck: disable_spellchecking() linter_exit_code = parsed_args.linter_exit_code - lint_report = LintReport() - fo = _output_styling(context.parsed_args, sys.stdout) + lint_report = initialize_lint_report(context) lint_context = gather_lint_info(context) for name_stem in LINTER_FORMATS: @@ -170,18 +198,17 @@ def perform_linting(context: CommandContext) -> None: if not os.path.isfile(filename): continue perform_linting_of_file( - fo, lint_context, filename, name_stem, context.parsed_args.auto_fix, lint_report, ) - if lint_report.diagnostics_without_severity: + if lint_report.number_of_invalid_diagnostics: _warn( "Some diagnostics did not explicitly set severity. Please report the bug and include the output" ) - if lint_report.diagnostic_errors: + if lint_report.number_of_broken_diagnostics: _error( "Some sub-linters reported issues. Please report the bug and include the output" ) @@ -193,6 +220,8 @@ def perform_linting(context: CommandContext) -> None: _info("only **partially** checked by this command at the time of writing.") _info("Please use `debputy check-manifest` to fully check the manifest.") + lint_report.finish_report() + if linter_exit_code: _exit_with_lint_code(lint_report) @@ -360,7 +389,6 @@ def _exit_with_lint_code(lint_report: LintReport) -> NoReturn: def perform_linting_of_file( - fo: OutputStylingBase, lint_context: LintContext, filename: str, file_format: str, @@ -375,7 +403,6 @@ def perform_linting_of_file( if auto_fixing_enabled: _auto_fix_run( - fo, lint_context, filename, text, @@ -384,7 +411,6 @@ def perform_linting_of_file( ) else: _diagnostics_run( - fo, lint_context, filename, text, @@ -406,7 +432,6 @@ def _edit_happens_before_last_fix( def _auto_fix_run( - fo: OutputStylingBase, lint_context: LintContext, filename: str, text: str, @@ -485,16 +510,13 @@ def _auto_fix_run( ) lines = text.splitlines(keepends=True) - for diagnostic in fixed_diagnostics: - report_diagnostic( - fo, - filename, - diagnostic, - lines, - True, - True, - lint_report, - ) + with lint_report.line_state(lint_state): + for diagnostic in fixed_diagnostics: + lint_report.report_diagnostic( + diagnostic, + result_state=LintDiagnosticResultState.FIXED, + ) + lint_state.content = text lint_state.lines = lines current_issues = linter(lint_state) @@ -506,68 +528,64 @@ def _auto_fix_run( os.chmod(output_filename, orig_mode) os.rename(output_filename, filename) lines = text.splitlines(keepends=True) + lint_state.content = text lint_state.lines = lines remaining_issues = linter(lint_state) or [] else: remaining_issues = current_issues or [] - for diagnostic in remaining_issues: - report_diagnostic( - fo, - filename, - diagnostic, - lines, - False, - False, - lint_report, - ) + with lint_report.line_state(lint_state): + for diagnostic in remaining_issues: + lint_report.report_diagnostic(diagnostic) - print() - if fixed_count: - remaining_issues_count = len(remaining_issues) - print( - fo.colored( - f"Fixes applied to {filename}: {fixed_count}." - f" Number of issues went from {issue_count_start} to {remaining_issues_count}", - fg="green", - style="bold", + if isinstance(lint_report, TermLintReport): + # TODO: Not optimal, but will do for now. + fo = lint_report.fo + print() + if fixed_count: + remaining_issues_count = len(remaining_issues) + print( + fo.colored( + f"Fixes applied to {filename}: {fixed_count}." + f" Number of issues went from {issue_count_start} to {remaining_issues_count}", + fg="green", + style="bold", + ) ) - ) - elif remaining_issues: - print( - fo.colored( - f"None of the issues in {filename} could be fixed automatically. Sorry!", - fg="yellow", - bg="black", - style="bold", + elif remaining_issues: + print( + fo.colored( + f"None of the issues in {filename} could be fixed automatically. Sorry!", + fg="yellow", + bg="black", + style="bold", + ) ) - ) - else: - assert not current_issues - print( - fo.colored( - f"No issues detected in {filename}", - fg="green", - style="bold", + else: + assert not current_issues + print( + fo.colored( + f"No issues detected in {filename}", + fg="green", + style="bold", + ) ) - ) - if too_many_rounds: - print( - fo.colored( - f"Not all fixes for issues in {filename} could be applied due to overlapping edits.", - fg="yellow", - bg="black", - style="bold", + if too_many_rounds: + print( + fo.colored( + f"Not all fixes for issues in {filename} could be applied due to overlapping edits.", + fg="yellow", + bg="black", + style="bold", + ) + ) + print( + "Running once more may cause more fixes to be applied. However, you may be facing" + " pathological performance." ) - ) - print( - "Running once more may cause more fixes to be applied. However, you may be facing" - " pathological performance." - ) def _diagnostics_run( - fo: OutputStylingBase, lint_context: LintContext, filename: str, text: str, @@ -576,29 +594,26 @@ def _diagnostics_run( ) -> None: lines = text.splitlines(keepends=True) lint_state = lint_context.state_for(filename, text, lines) - issues = linter(lint_state) or [] - for diagnostic in issues: - actions = provide_standard_quickfixes_from_diagnostics( - CodeActionParams( - TextDocumentIdentifier(filename), - diagnostic.range, - CodeActionContext( - [diagnostic], + with lint_report.line_state(lint_state): + issues = linter(lint_state) or [] + for diagnostic in issues: + actions = provide_standard_quickfixes_from_diagnostics( + CodeActionParams( + TextDocumentIdentifier(filename), + diagnostic.range, + CodeActionContext( + [diagnostic], + ), ), - ), - ) - auto_fixer = resolve_auto_fixer(filename, actions) - has_auto_fixer = bool(auto_fixer) + ) + auto_fixer = resolve_auto_fixer(filename, actions) + has_auto_fixer = bool(auto_fixer) - report_diagnostic( - fo, - filename, - diagnostic, - lines, - has_auto_fixer, - False, - lint_report, - ) + result_state = LintDiagnosticResultState.REPORTED + if has_auto_fixer: + result_state = LintDiagnosticResultState.FIXABLE + + lint_report.report_diagnostic(diagnostic, result_state=result_state) def resolve_auto_fixer( diff --git a/src/debputy/linting/lint_report_junit.py b/src/debputy/linting/lint_report_junit.py new file mode 100644 index 0000000..8b1d635 --- /dev/null +++ b/src/debputy/linting/lint_report_junit.py @@ -0,0 +1,75 @@ +import textwrap +from typing import Iterable, TYPE_CHECKING + +from debputy.util import _info + +if TYPE_CHECKING: + from junit_xml import TestSuite, TestCase, to_xml_report_file +else: + try: + from junit_xml import TestSuite, TestCase, to_xml_report_file + except ImportError: + pass + + +from debputy.linting.lint_util import ( + LintReport, + LintDiagnosticResult, + LintDiagnosticResultState, + debputy_severity, +) + + +class JunitLintReport(LintReport): + + def __init__(self, output_filename: str) -> None: + super().__init__() + self._output_filename = output_filename + + def finish_report(self) -> None: + # Nothing to do for now + all_test_cases = list(self._as_test_cases()) + test_suites = [ + TestSuite( + "debputy lint", + test_cases=all_test_cases, + timestamp=str(self.start_timestamp), + ) + ] + with open(self._output_filename, "w", encoding="utf-8") as wfd: + to_xml_report_file(wfd, test_suites, encoding="utf-8") + _info(f"Wrote {self._output_filename}") + + def _as_test_cases(self) -> Iterable["TestCase"]: + for filename, duration in self.durations.items(): + results = self.diagnostics_by_file.get(filename, []) + yield self._as_test_case(filename, results, duration) + + def _as_test_case( + self, + filename: str, + diagnostic_results: Iterable[LintDiagnosticResult], + duration: float, + ) -> "TestCase": + case = TestCase( + filename, + # The JUnit schema has `classname` as mandatory + classname=filename, + allow_multiple_subelements=True, + elapsed_sec=duration, + ) + for diagnostic_result in diagnostic_results: + if diagnostic_result.result_state == LintDiagnosticResultState.FIXED: + continue + diagnostic = diagnostic_result.diagnostic + severity = debputy_severity(diagnostic) + output = textwrap.dedent( + f"""\ + {filename} [{severity}] ({diagnostic.range}): {diagnostic.message} + """ + ) + case.add_failure_info( + message=f"[{severity}]: {diagnostic.message}", + output=output, + ) + return case diff --git a/src/debputy/linting/lint_util.py b/src/debputy/linting/lint_util.py index 745d24c..1fde104 100644 --- a/src/debputy/linting/lint_util.py +++ b/src/debputy/linting/lint_util.py @@ -1,24 +1,31 @@ +import collections +import contextlib import dataclasses +import datetime import os +import time +import typing +from collections import defaultdict, Counter +from enum import IntEnum from typing import ( List, Optional, Callable, - Counter, TYPE_CHECKING, Mapping, Sequence, cast, ) +from lsprotocol.types import Position, Range, Diagnostic, DiagnosticSeverity, TextEdit + from debputy.commands.debputy_cmd.output import OutputStylingBase from debputy.filesystem_scan import VirtualPathBase from debputy.lsp.diagnostics import LintSeverity from debputy.lsp.vendoring._deb822_repro import Deb822FileElement, parse_deb822_file from debputy.packages import SourcePackage, BinaryPackage from debputy.plugin.api.feature_set import PluginProvidedFeatureSet -from debputy.util import _DEFAULT_LOGGER, _warn -from lsprotocol.types import Position, Range, Diagnostic, DiagnosticSeverity, TextEdit +from debputy.util import _warn if TYPE_CHECKING: from debputy.lsp.text_util import LintCapablePositionCodec @@ -124,48 +131,130 @@ class LintStateImpl(LintState): return cache -@dataclasses.dataclass(slots=True) -class LintReport: - diagnostics_count: Counter[DiagnosticSeverity] = dataclasses.field( - default_factory=Counter - ) - diagnostics_without_severity: int = 0 - diagnostic_errors: int = 0 - fixed: int = 0 - fixable: int = 0 +class LintDiagnosticResultState(IntEnum): + REPORTED = 1 + FIXABLE = 2 + FIXED = 3 -class LinterPositionCodec: - - def client_num_units(self, chars: str): - return len(chars) - - def position_from_client_units( - self, lines: List[str], position: Position - ) -> Position: +@dataclasses.dataclass(slots=True, frozen=True) +class LintDiagnosticResult: + diagnostic: Diagnostic + result_state: LintDiagnosticResultState + invalid_marker: Optional[RuntimeError] + is_file_level_diagnostic: bool + has_broken_range: bool + missing_severity: bool - if len(lines) == 0: - return Position(0, 0) - if position.line >= len(lines): - return Position(len(lines) - 1, self.client_num_units(lines[-1])) - return position - def position_to_client_units( - self, _lines: List[str], position: Position - ) -> Position: - return position +class LintReport: - def range_from_client_units(self, _lines: List[str], range: Range) -> Range: - return range + def __init__(self) -> None: + self.diagnostics_count: typing.Counter[DiagnosticSeverity] = Counter() + self.diagnostics_by_file: Mapping[str, List[LintDiagnosticResult]] = ( + defaultdict(list) + ) + self.number_of_invalid_diagnostics: int = 0 + self.number_of_broken_diagnostics: int = 0 + self.lint_state: Optional[LintState] = None + self.start_timestamp = datetime.datetime.now() + self.durations: Mapping[str, float] = collections.defaultdict(lambda: 0.0) + self._timer = time.perf_counter() + + @contextlib.contextmanager + def line_state(self, lint_state: LintState) -> typing.Iterable[None]: + previous = self.lint_state + if previous is not None: + path = previous.path + duration = time.perf_counter() - self._timer + self.durations[path] += duration + + self.lint_state = lint_state + + try: + self._timer = time.perf_counter() + yield + finally: + now = time.perf_counter() + duration = now - self._timer + self.durations[lint_state.path] += duration + self._timer = now + self.lint_state = previous + + def report_diagnostic( + self, + diagnostic: Diagnostic, + *, + result_state: LintDiagnosticResultState = LintDiagnosticResultState.REPORTED, + in_file: Optional[str] = None, + ) -> None: + lint_state = self.lint_state + assert lint_state is not None + if in_file is None: + in_file = lint_state.path + severity = diagnostic.severity + missing_severity = False + error_marker: Optional[RuntimeError] = None + if severity is None: + self.number_of_invalid_diagnostics += 1 + severity = DiagnosticSeverity.Warning + diagnostic.severity = severity + missing_severity = True + + lines = lint_state.lines + diag_range = diagnostic.range + start_pos = diag_range.start + end_pos = diag_range.start + is_file_level_diagnostic = _is_file_level_diagnostic( + lines, + start_pos.line, + start_pos.character, + end_pos.line, + end_pos.character, + ) + has_broken_range = not is_file_level_diagnostic and ( + end_pos.line > len(lines) or start_pos.line < 0 + ) - def range_to_client_units(self, _lines: List[str], range: Range) -> Range: - return range + if has_broken_range or missing_severity: + error_marker = RuntimeError("Registration Marker for invalid diagnostic") + diagnostic_result = LintDiagnosticResult( + diagnostic, + result_state, + error_marker, + is_file_level_diagnostic, + has_broken_range, + missing_severity, + ) -LINTER_POSITION_CODEC = LinterPositionCodec() + self.diagnostics_by_file[in_file].append(diagnostic_result) + self.diagnostics_count[severity] += 1 + self.process_diagnostic(in_file, lint_state, diagnostic_result) + + def process_diagnostic( + self, + filename: str, + lint_state: LintState, + diagnostic_result: LintDiagnosticResult, + ) -> None: + # Subclass hook + pass + + def finish_report(self) -> None: + # Subclass hook + pass + + +_LS2DEBPUTY_SEVERITY: Mapping[DiagnosticSeverity, LintSeverity] = { + DiagnosticSeverity.Error: "error", + DiagnosticSeverity.Warning: "warning", + DiagnosticSeverity.Information: "informational", + DiagnosticSeverity.Hint: "pedantic", +} -_SEVERITY2TAG = { +_TERM_SEVERITY2TAG = { DiagnosticSeverity.Error: lambda fo, lint_tag=None: fo.colored( lint_tag if lint_tag else "error", fg="red", @@ -193,6 +282,117 @@ _SEVERITY2TAG = { } +def debputy_severity(diagnostic: Diagnostic) -> LintSeverity: + lint_tag: Optional[LintSeverity] = None + if isinstance(diagnostic.data, dict): + lint_tag = cast("LintSeverity", diagnostic.data.get("lint_severity")) + + if lint_tag is not None: + return lint_tag + severity = diagnostic.severity + if severity is None: + return "warning" + return _LS2DEBPUTY_SEVERITY.get(severity, "warning") + + +class TermLintReport(LintReport): + + def __init__(self, fo: OutputStylingBase) -> None: + super().__init__() + self.fo = fo + + def finish_report(self) -> None: + # Nothing to do for now + pass + + def process_diagnostic( + self, + filename: str, + lint_state: LintState, + diagnostic_result: LintDiagnosticResult, + ) -> None: + diagnostic = diagnostic_result.diagnostic + fo = self.fo + severity = diagnostic.severity + assert severity is not None + if diagnostic_result.result_state != LintDiagnosticResultState.FIXED: + tag_unresolved = _TERM_SEVERITY2TAG[severity] + lint_tag: Optional[LintSeverity] = debputy_severity(diagnostic) + tag = tag_unresolved(fo, lint_tag) + else: + tag = fo.colored( + "auto-fixing", + fg="magenta", + bg="black", + style="bold", + ) + start_line = diagnostic.range.start.line + start_position = diagnostic.range.start.character + end_line = diagnostic.range.end.line + end_position = diagnostic.range.end.character + has_fixit = "" + lines = lint_state.lines + line_no_width = len(str(len(lines))) + if diagnostic_result.result_state == LintDiagnosticResultState.FIXABLE: + has_fixit = " [Correctable via --auto-fix]" + print( + f"{tag}: File: {filename}:{start_line+1}:{start_position}:{end_line+1}:{end_position}: {diagnostic.message}{has_fixit}", + ) + if diagnostic_result.missing_severity: + _warn( + " This warning did not have an explicit severity; Used Warning as a fallback!" + ) + if diagnostic_result.result_state == LintDiagnosticResultState.FIXED: + # If it is fixed, there is no reason to show additional context. + return + if diagnostic_result.is_file_level_diagnostic: + print(" File-level diagnostic") + return + if diagnostic_result.has_broken_range: + _warn( + "Bug in the underlying linter: The line numbers of the warning does not fit in the file..." + ) + return + lines_to_print = _lines_to_print(diagnostic.range) + if lines_to_print == 1: + line = _highlight_range(fo, lines[start_line], start_line, diagnostic.range) + print(f" {start_line+1:{line_no_width}}: {line}") + else: + for line_no in range(start_line, end_line): + line = _highlight_range(fo, lines[line_no], line_no, diagnostic.range) + print(f" {line_no+1:{line_no_width}}: {line}") + + +class LinterPositionCodec: + + def client_num_units(self, chars: str): + return len(chars) + + def position_from_client_units( + self, lines: List[str], position: Position + ) -> Position: + + if len(lines) == 0: + return Position(0, 0) + if position.line >= len(lines): + return Position(len(lines) - 1, self.client_num_units(lines[-1])) + return position + + def position_to_client_units( + self, _lines: List[str], position: Position + ) -> Position: + return position + + def range_from_client_units(self, _lines: List[str], range: Range) -> Range: + return range + + def range_to_client_units(self, _lines: List[str], range: Range) -> Range: + return range + + +LINTER_POSITION_CODEC = LinterPositionCodec() + + def _lines_to_print(range_: Range) -> int: count = range_.end.line - range_.start.line if range_.end.character > 0: @@ -221,81 +421,6 @@ def _highlight_range( return prefix + marked_part + suffix -def report_diagnostic( - fo: OutputStylingBase, - filename: str, - diagnostic: Diagnostic, - lines: List[str], - auto_fixable: bool, - auto_fixed: bool, - lint_report: LintReport, -) -> None: - logger = _DEFAULT_LOGGER - assert logger is not None - severity = diagnostic.severity - missing_severity = False - if severity is None: - severity = DiagnosticSeverity.Warning - missing_severity = True - if not auto_fixed: - tag_unresolved = _SEVERITY2TAG.get(severity) - lint_tag: Optional[LintSeverity] = None - if isinstance(diagnostic.data, dict): - lint_tag = cast("LintSeverity", diagnostic.data.get("lint_severity")) - if tag_unresolved is None: - tag_unresolved = _SEVERITY2TAG[DiagnosticSeverity.Warning] - lint_report.diagnostics_without_severity += 1 - else: - lint_report.diagnostics_count[severity] += 1 - tag = tag_unresolved(fo, lint_tag) - else: - tag = fo.colored( - "auto-fixing", - fg="magenta", - bg="black", - style="bold", - ) - start_line = diagnostic.range.start.line - start_position = diagnostic.range.start.character - end_line = diagnostic.range.end.line - end_position = diagnostic.range.end.character - has_fixit = "" - line_no_width = len(str(len(lines))) - if not auto_fixed and auto_fixable: - has_fixit = " [Correctable via --auto-fix]" - lint_report.fixable += 1 - print( - f"{tag}: File: {filename}:{start_line+1}:{start_position}:{end_line+1}:{end_position}: {diagnostic.message}{has_fixit}", - ) - if missing_severity: - _warn( - " This warning did not have an explicit severity; Used Warning as a fallback!" - ) - if auto_fixed: - # If it is fixed, there is no reason to show additional context. - lint_report.fixed += 1 - return - if _is_file_level_diagnostic( - lines, start_line, start_position, end_line, end_position - ): - print(f" File-level diagnostic") - return - lines_to_print = _lines_to_print(diagnostic.range) - if end_line > len(lines) or start_line < 0: - lint_report.diagnostic_errors += 1 - _warn( - "Bug in the underlying linter: The line numbers of the warning does not fit in the file..." - ) - return - if lines_to_print == 1: - line = _highlight_range(fo, lines[start_line], start_line, diagnostic.range) - print(f" {start_line+1:{line_no_width}}: {line}") - else: - for line_no in range(start_line, end_line): - line = _highlight_range(fo, lines[line_no], line_no, diagnostic.range) - print(f" {line_no+1:{line_no_width}}: {line}") - - def _is_file_level_diagnostic( lines: List[str], start_line: int, diff --git a/src/debputy/lsp/diagnostics.py b/src/debputy/lsp/diagnostics.py index 5ae7ec5..618b91d 100644 --- a/src/debputy/lsp/diagnostics.py +++ b/src/debputy/lsp/diagnostics.py @@ -1,6 +1,6 @@ from typing import TypedDict, NotRequired, List, Any, Literal, Optional -LintSeverity = Literal["spelling"] +LintSeverity = Literal["error", "warning", "informational", "pedantic", "spelling"] class DiagnosticData(TypedDict): diff --git a/src/debputy/lsp/lsp_debian_changelog.py b/src/debputy/lsp/lsp_debian_changelog.py index 2deff97..277b06e 100644 --- a/src/debputy/lsp/lsp_debian_changelog.py +++ b/src/debputy/lsp/lsp_debian_changelog.py @@ -253,6 +253,7 @@ def _scan_debian_changelog_for_diagnostics( lines = lint_state.lines position_codec = lint_state.position_codec entry_no = 0 + entry_limit = 2 for line_no, line in enumerate(lines): orig_line = line line = line.rstrip() @@ -264,6 +265,10 @@ def _scan_debian_changelog_for_diagnostics( if not line.startswith(" "): if not line[0].isspace(): entry_no += 1 + # Figure out the right cut which may not be as simple as just the + # top two. + if entry_no > entry_limit: + break diagnostics.extend( _check_header_line( lint_state, diff --git a/src/debputy/lsp/lsp_debian_control.py b/src/debputy/lsp/lsp_debian_control.py index ce92374..fd8598e 100644 --- a/src/debputy/lsp/lsp_debian_control.py +++ b/src/debputy/lsp/lsp_debian_control.py @@ -360,14 +360,41 @@ def _custom_hover( return None content = line[col_idx + 1 :].strip() + # Synopsis return textwrap.dedent( f"""\ # Package synopsis - The synopsis is a single line "noun phrase" description of the package. - It is typically used in search results and other cases where a - user-interface has limited space for text. + The synopsis functions as a phrase describing the package, not a + complete sentence, so sentential punctuation is inappropriate: it + does not need extra capital letters or a final period (full stop). + It should also omit any initial indefinite or definite article + - "a", "an", or "the". Thus for instance: + + ``` + Package: libeg0 + Description: exemplification support library + ``` + + Technically this is a noun phrase minus articles, as opposed to a + verb phrase. A good heuristic is that it should be possible to + substitute the package name and synopsis into this formula: + + ``` + # Generic + The package provides {{a,an,the,some}} synopsis. + + # The current package for comparison + The package provides {{a,an,the,some}} {content}. + ``` + + Other advice for writing synopsis: + * Avoid using the package name. Any software would display the + package name already and it generally does not help the user + understand what they are looking at. + * In many situations, the user will only see the package name + and its synopsis. The synopsis must standalone. **Example renderings in various terminal UIs**: ``` @@ -379,19 +406,34 @@ def _custom_hover( package - {content} ``` - Advice for writing synopsis: - * Avoid using the package name. Any software would display the - package name already and it generally does not help the user - understand what they are looking at. - * In many situations, the user will only see the package name - and its synopsis. The synopsis must standalone. - * When writing the synopsis, it is often a good idea to write - it such that it fits into the sentence like "This package - provides [a|an|the] ..." (see below). + ## Reference example + + An reference example for comparison: The Sphinx package + (python3-sphinx/7.2.6-6) had the following synopsis: + + ``` + Description: documentation generator for Python projects + ``` + + In the test sentence, it would read as: - **Phrasing test**: ``` - This package provides [a|an|the] {content}. + The python3-sphinx package provides a documentation generator for Python projects. + ``` + + **Side-by-side comparison in the terminal UIs**: + ``` + # apt search TERM + python3-sphinx/stable,now 7.2.6-6 all: + documentation generator for Python projects + + package/stable,now 1.0-1 all: + {content} + + + # apt-get search TERM + package - {content} + python3-sphinx - documentation generator for Python projects ``` """ ) @@ -539,6 +581,7 @@ def _binary_package_checks( def _diagnostics_for_paragraph( + deb822_file: Deb822FileElement, stanza: Deb822ParagraphElement, stanza_position: "TEPosition", source_stanza: Deb822ParagraphElement, @@ -558,19 +601,17 @@ def _diagnostics_for_paragraph( te_range_to_lsp(representation_field_range), ) for known_field in known_fields.values(): - missing_field_severity = known_field.missing_field_severity - if missing_field_severity is None or known_field.name in stanza: - continue - - if known_field.inherits_from_source and known_field.name in source_stanza: + if known_field.name in stanza: continue - diagnostics.append( - Diagnostic( + diagnostics.extend( + known_field.field_omitted_diagnostics( + deb822_file, representation_field_range, - f"Stanza is missing field {known_field.name}", - severity=missing_field_severity, - source="debputy", + stanza, + stanza_position, + source_stanza, + lint_state, ) ) @@ -678,6 +719,7 @@ def _diagnostics_for_paragraph( continue diagnostics.extend( known_field.field_diagnostics( + deb822_file, kvpair, stanza, stanza_position, @@ -892,6 +934,7 @@ def _lint_debian_control( known_fields = SOURCE_FIELDS other_known_fields = BINARY_FIELDS _diagnostics_for_paragraph( + deb822_file, paragraph, paragraph_pos, source_paragraph, diff --git a/src/debputy/lsp/lsp_debian_control_reference_data.py b/src/debputy/lsp/lsp_debian_control_reference_data.py index 5b2e5f3..7a0fbdb 100644 --- a/src/debputy/lsp/lsp_debian_control_reference_data.py +++ b/src/debputy/lsp/lsp_debian_control_reference_data.py @@ -25,9 +25,6 @@ from typing import ( Sequence, ) -from debputy.filesystem_scan import VirtualPathBase -from debputy.linting.lint_util import LintState -from debputy.lsp.vendoring._deb822_repro.types import TE from debian.debian_support import DpkgArchTable, Version from lsprotocol.types import ( DiagnosticSeverity, @@ -43,6 +40,8 @@ from lsprotocol.types import ( CompletionItemKind, ) +from debputy.filesystem_scan import VirtualPathBase +from debputy.linting.lint_util import LintState from debputy.lsp.diagnostics import DiagnosticData from debputy.lsp.lsp_reference_keyword import ( ALL_PUBLIC_NAMED_STYLES, @@ -76,6 +75,7 @@ from debputy.lsp.vendoring._deb822_repro.parsing import ( _parse_whitespace_list_value, parse_deb822_file, Deb822ParsedTokenList, + Deb822ValueLineElement, ) from debputy.lsp.vendoring._deb822_repro.tokens import ( Deb822FieldNameToken, @@ -86,6 +86,7 @@ from debputy.lsp.vendoring._deb822_repro.tokens import ( Deb822SpaceSeparatorToken, ) from debputy.lsp.vendoring._deb822_repro.types import FormatterCallback +from debputy.lsp.vendoring._deb822_repro.types import TE from debputy.lsp.vendoring.wrap_and_sort import _sort_packages_key from debputy.path_matcher import BasenameGlobMatch from debputy.plugin.api import VirtualPath @@ -111,7 +112,12 @@ S = TypeVar("S", bound="StanzaMetadata") # FIXME: should go into python3-debian _RE_COMMA = re.compile("([^,]*),([^,]*)") +_RE_SYNOPSIS_STARTS_WITH_ARTICLE = re.compile(r"^\s*(an?|the)(?:\s|$)", re.I) _RE_SV = re.compile(r"(\d+[.]\d+[.]\d+)([.]\d+)?") +_RE_SYNOPSIS_IS_TEMPLATE = re.compile( + r"^\s*(missing|<insert up to \d+ chars description>)$" +) +_RE_SYNOPSIS_IS_TOO_SHORT = re.compile(r"^\s*(\S+)$") CURRENT_STANDARDS_VERSION = Version("4.7.0") @@ -151,6 +157,7 @@ LIST_COMMA_OR_SPACE_SEPARATED_INTERPRETATION = ListInterpretation( CustomFieldCheck = Callable[ [ "F", + Deb822FileElement, Deb822KeyValuePairElement, "TERange", Deb822ParagraphElement, @@ -345,7 +352,9 @@ ALL_PRIORITIES = allowed_values( ) -def all_architectures_and_wildcards(arch2table) -> Iterable[Union[str, Keyword]]: +def all_architectures_and_wildcards( + arch2table, *, allow_negations: bool = False +) -> Iterable[Union[str, Keyword]]: wildcards = set() yield Keyword( "any", @@ -375,21 +384,32 @@ def all_architectures_and_wildcards(arch2table) -> Iterable[Union[str, Keyword]] ) for arch_name, quad_tuple in arch2table.items(): yield arch_name + if allow_negations: + yield f"!{arch_name}" cpu_wc = "any-" + quad_tuple.cpu_name os_wc = quad_tuple.os_name + "-any" if cpu_wc not in wildcards: yield cpu_wc + if allow_negations: + yield f"!{cpu_wc}" wildcards.add(cpu_wc) if os_wc not in wildcards: yield os_wc + if allow_negations: + yield f"!{os_wc}" wildcards.add(os_wc) # Add the remaining wildcards @functools.lru_cache -def dpkg_arch_and_wildcards() -> FrozenSet[Union[str, Keyword]]: +def dpkg_arch_and_wildcards(*, allow_negations=False) -> FrozenSet[Union[str, Keyword]]: dpkg_arch_table = DpkgArchTable.load_arch_table() - return frozenset(all_architectures_and_wildcards(dpkg_arch_table._arch2table)) + return frozenset( + all_architectures_and_wildcards( + dpkg_arch_table._arch2table, + allow_negations=allow_negations, + ) + ) def extract_first_value_and_position( @@ -422,6 +442,7 @@ def extract_first_value_and_position( def _sv_field_validation( _known_field: "F", + _deb822_file: Deb822FileElement, kvpair: Deb822KeyValuePairElement, _field_range: "TERange", _stanza: Deb822ParagraphElement, @@ -478,6 +499,7 @@ def _sv_field_validation( def _dctrl_ma_field_validation( _known_field: "F", + _deb822_file: Deb822FileElement, _kvpair: Deb822KeyValuePairElement, _field_range: "TERange", stanza: Deb822ParagraphElement, @@ -503,6 +525,7 @@ def _dctrl_ma_field_validation( def _udeb_only_field_validation( known_field: "F", + _deb822_file: Deb822FileElement, _kvpair: Deb822KeyValuePairElement, field_range_te: "TERange", stanza: Deb822ParagraphElement, @@ -549,6 +572,7 @@ def _complete_only_for_udeb_pkgs( def _arch_not_all_only_field_validation( known_field: "F", + _deb822_file: Deb822FileElement, _kvpair: Deb822KeyValuePairElement, field_range_te: "TERange", stanza: Deb822ParagraphElement, @@ -570,6 +594,140 @@ def _arch_not_all_only_field_validation( ) +def _span_to_client_range( + span: Tuple[int, int], + relative_to: "TEPosition", + lint_state: LintState, +) -> Range: + range_server_units = Range( + Position( + relative_to.line_position, + relative_to.cursor_position + span[0], + ), + Position( + relative_to.line_position, + relative_to.cursor_position + span[1], + ), + ) + return lint_state.position_codec.range_to_client_units( + lint_state.lines, + range_server_units, + ) + + +def _check_synopsis( + synopsis_value_line: Deb822ValueLineElement, + synopsis_range_te: "TERange", + field_name_range_te: "TERange", + package: Optional[str], + lint_state: LintState, +) -> Iterable[Diagnostic]: + # This function would compute range would be wrong if there is a comment + assert synopsis_value_line.comment_element is None + synopsis_text_with_leading_space = synopsis_value_line.convert_to_text().rstrip() + if not synopsis_text_with_leading_space: + yield Diagnostic( + lint_state.position_codec.range_to_client_units( + lint_state.lines, + te_range_to_lsp(field_name_range_te), + ), + "Package synopsis is missing", + severity=DiagnosticSeverity.Warning, + source="debputy", + ) + return + synopsis_text_trimmed = synopsis_text_with_leading_space.lstrip() + synopsis_offset = len(synopsis_text_with_leading_space) - len(synopsis_text_trimmed) + # synopsis_text_trimmed_lower = synopsis_text_trimmed.lower() + starts_with_article = _RE_SYNOPSIS_STARTS_WITH_ARTICLE.search( + synopsis_text_with_leading_space + ) + # TODO: Handle ${...} expansion + if starts_with_article: + yield Diagnostic( + _span_to_client_range( + starts_with_article.span(1), + synopsis_range_te.start_pos, + lint_state, + ), + "Package synopsis starts with an article (a/an/the).", + severity=DiagnosticSeverity.Warning, + source="DevRef 6.2.2", + ) + if len(synopsis_text_trimmed) >= 80: + # Policy says `certainly under 80 characters.`, so exactly 80 characters is considered bad too. + span = synopsis_offset + 79, len(synopsis_text_with_leading_space) + yield Diagnostic( + _span_to_client_range( + span, + synopsis_range_te.start_pos, + lint_state, + ), + "Package synopsis is too long.", + severity=DiagnosticSeverity.Warning, + source="Policy 3.4.1", + ) + if template_match := _RE_SYNOPSIS_IS_TEMPLATE.match( + synopsis_text_with_leading_space + ): + yield Diagnostic( + _span_to_client_range( + template_match.span(1), + synopsis_range_te.start_pos, + lint_state, + ), + "Package synopsis is a placeholder", + severity=DiagnosticSeverity.Warning, + source="debputy", + ) + elif too_short_match := _RE_SYNOPSIS_IS_TOO_SHORT.match( + synopsis_text_with_leading_space + ): + yield Diagnostic( + _span_to_client_range( + too_short_match.span(1), + synopsis_range_te.start_pos, + lint_state, + ), + "Package synopsis is too short", + severity=DiagnosticSeverity.Warning, + source="debputy", + ) + + +def dctrl_description_validator( + _known_field: "F", + _deb822_file: Deb822FileElement, + kvpair: Deb822KeyValuePairElement, + field_range_te: "TERange", + stanza: Deb822ParagraphElement, + _stanza_position: "TEPosition", + lint_state: LintState, +) -> Iterable[Diagnostic]: + value_lines = kvpair.value_element.value_lines + if not value_lines: + return + package = stanza.get("Package") + synopsis_value_line = value_lines[0] + value_range_te = kvpair.value_element.range_in_parent().relative_to( + field_range_te.start_pos + ) + if synopsis_value_line.continuation_line_token is None: + field_name_range_te = kvpair.field_token.range_in_parent().relative_to( + field_range_te.start_pos + ) + synopsis_range_te = synopsis_value_line.range_in_parent().relative_to( + value_range_te.start_pos + ) + yield from _check_synopsis( + synopsis_value_line, + synopsis_range_te, + field_name_range_te, + package, + lint_state, + ) + + def _each_value_match_regex_validation( regex: re.Pattern, *, @@ -578,6 +736,7 @@ def _each_value_match_regex_validation( def _validator( _known_field: "F", + _deb822_file: Deb822FileElement, kvpair: Deb822KeyValuePairElement, field_range_te: "TERange", _stanza: Deb822ParagraphElement, @@ -707,6 +866,7 @@ def _dep5_unnecessary_symbols( def _dep5_files_check( known_field: "F", + _deb822_file: Deb822FileElement, kvpair: Deb822KeyValuePairElement, field_range_te: "TERange", _stanza: Deb822ParagraphElement, @@ -740,6 +900,7 @@ def _dep5_files_check( def _combined_custom_field_check(*checks: CustomFieldCheck) -> CustomFieldCheck: def _validator( known_field: "F", + deb822_file: Deb822FileElement, kvpair: Deb822KeyValuePairElement, field_range_te: "TERange", stanza: Deb822ParagraphElement, @@ -749,6 +910,7 @@ def _combined_custom_field_check(*checks: CustomFieldCheck) -> CustomFieldCheck: for check in checks: yield from check( known_field, + deb822_file, kvpair, field_range_te, stanza, @@ -1210,8 +1372,29 @@ class Deb822KnownField: if keyword.is_keyword_valid_completion_in_stanza(stanza_parts) ] + def field_omitted_diagnostics( + self, + deb822_file: Deb822FileElement, + representation_field_range: Range, + stanza: Deb822ParagraphElement, + stanza_position: "TEPosition", + header_stanza: Optional[Deb822FileElement], + lint_state: LintState, + ) -> Iterable[Diagnostic]: + missing_field_severity = self.missing_field_severity + if missing_field_severity is None: + return + + yield Diagnostic( + representation_field_range, + f"Stanza is missing field {self.name}", + severity=missing_field_severity, + source="debputy", + ) + def field_diagnostics( self, + deb822_file: Deb822FileElement, kvpair: Deb822KeyValuePairElement, stanza: Deb822ParagraphElement, stanza_position: "TEPosition", @@ -1233,6 +1416,7 @@ class Deb822KnownField: if self.custom_field_check is not None: yield from self.custom_field_check( self, + deb822_file, kvpair, field_range_te, stanza, @@ -1571,6 +1755,38 @@ class DTestsCtrlKnownField(DctrlLikeKnownField): class DctrlKnownField(DctrlLikeKnownField): inherits_from_source: bool = False + def field_omitted_diagnostics( + self, + deb822_file: Deb822FileElement, + representation_field_range: Range, + stanza: Deb822ParagraphElement, + stanza_position: "TEPosition", + header_stanza: Optional[Deb822FileElement], + lint_state: LintState, + ) -> Iterable[Diagnostic]: + missing_field_severity = self.missing_field_severity + if missing_field_severity is None: + return + + if ( + self.inherits_from_source + and header_stanza is not None + and self.name in header_stanza + ): + return + + if self.name == "Standards-Version": + stanzas = list(deb822_file)[1:] + if all(s.get("Package-Type") == "udeb" for s in stanzas): + return + + yield Diagnostic( + representation_field_range, + f"Stanza is missing field {self.name}", + severity=missing_field_severity, + source="debputy", + ) + def reformat_field( self, effective_preference: "EffectivePreference", @@ -1643,6 +1859,7 @@ SOURCE_FIELDS = _fields( DctrlKnownField( "Standards-Version", FieldValueClass.SINGLE_VALUE, + # Conditionally mandatory (special-cased by field_omitted_diagnostics) missing_field_severity=DiagnosticSeverity.Error, custom_field_check=_sv_field_validation, synopsis_doc="Debian Policy version this package complies with", @@ -2247,6 +2464,7 @@ BINARY_FIELDS = _fields( FieldValueClass.SPACE_SEPARATED_LIST, missing_field_severity=DiagnosticSeverity.Error, unknown_value_diagnostic_severity=None, + # FIXME: Specialize validation for architecture ("!foo" is not a "typo" and should have a better warning) known_values=allowed_values(*dpkg_arch_and_wildcards()), synopsis_doc="Architecture of the package", hover_text=textwrap.dedent( @@ -3027,6 +3245,7 @@ BINARY_FIELDS = _fields( # It will build just fine. But no one will know what it is for, so it probably won't be installed missing_field_severity=DiagnosticSeverity.Warning, synopsis_doc="Package synopsis and description", + custom_field_check=dctrl_description_validator, hover_text=textwrap.dedent( """\ A human-readable description of the package. This field consists of two related but distinct parts. @@ -3558,7 +3777,8 @@ _DTESTSCTRL_FIELDS = _fields( "Architecture", FieldValueClass.SPACE_SEPARATED_LIST, unknown_value_diagnostic_severity=None, - known_values=allowed_values(*dpkg_arch_and_wildcards()), + # FIXME: Specialize validation for architecture ("!fou" to "foo" would be bad) + known_values=allowed_values(*dpkg_arch_and_wildcards(allow_negations=True)), synopsis_doc="Only run these tests on specific architectures", hover_text=textwrap.dedent( """\ diff --git a/src/debputy/lsp/lsp_debian_copyright.py b/src/debputy/lsp/lsp_debian_copyright.py index 5895669..ad454ba 100644 --- a/src/debputy/lsp/lsp_debian_copyright.py +++ b/src/debputy/lsp/lsp_debian_copyright.py @@ -136,6 +136,7 @@ def _paragraph_representation_field( def _diagnostics_for_paragraph( + deb822_file: Deb822FileElement, stanza: Deb822ParagraphElement, stanza_position: "TEPosition", known_fields: Mapping[str, Deb822KnownField], @@ -159,16 +160,17 @@ def _diagnostics_for_paragraph( representation_field_range_server_units, ) for known_field in known_fields.values(): - missing_field_severity = known_field.missing_field_severity - if missing_field_severity is None or known_field.name in stanza: + if known_field.name in stanza: continue - diagnostics.append( - Diagnostic( + diagnostics.extend( + known_field.field_omitted_diagnostics( + deb822_file, representation_field_range, - f"Stanza is missing field {known_field.name}", - severity=missing_field_severity, - source="debputy", + stanza, + stanza_position, + None, + lint_state, ) ) @@ -261,6 +263,7 @@ def _diagnostics_for_paragraph( continue diagnostics.extend( known_field.field_diagnostics( + deb822_file, kvpair, stanza, stanza_position, @@ -464,6 +467,7 @@ def _lint_debian_copyright( else: break _diagnostics_for_paragraph( + deb822_file, paragraph, paragraph_pos, known_fields, diff --git a/src/debputy/lsp/lsp_debian_tests_control.py b/src/debputy/lsp/lsp_debian_tests_control.py index 3d418cb..5517b52 100644 --- a/src/debputy/lsp/lsp_debian_tests_control.py +++ b/src/debputy/lsp/lsp_debian_tests_control.py @@ -134,6 +134,7 @@ def _paragraph_representation_field( def _diagnostics_for_paragraph( + deb822_file: Deb822FileElement, stanza: Deb822ParagraphElement, stanza_position: "TEPosition", known_fields: Mapping[str, Deb822KnownField], @@ -155,16 +156,17 @@ def _diagnostics_for_paragraph( representation_field_range_server_units, ) for known_field in known_fields.values(): - missing_field_severity = known_field.missing_field_severity - if missing_field_severity is None or known_field.name in stanza: + if known_field.name in stanza: continue - diagnostics.append( - Diagnostic( + diagnostics.extend( + known_field.field_omitted_diagnostics( + deb822_file, representation_field_range, - f"Stanza is missing field {known_field.name}", - severity=missing_field_severity, - source="debputy", + stanza, + stanza_position, + None, + lint_state, ) ) @@ -261,6 +263,7 @@ def _diagnostics_for_paragraph( continue diagnostics.extend( known_field.field_diagnostics( + deb822_file, kvpair, stanza, stanza_position, @@ -451,6 +454,7 @@ def _lint_debian_tests_control( break known_fields = _DTESTSCTRL_FIELDS _diagnostics_for_paragraph( + deb822_file, paragraph, paragraph_pos, known_fields, diff --git a/src/debputy/lsp/style-preferences.yaml b/src/debputy/lsp/style-preferences.yaml index 6e76d2a..982f242 100644 --- a/src/debputy/lsp/style-preferences.yaml +++ b/src/debputy/lsp/style-preferences.yaml @@ -10,7 +10,9 @@ formatting: trailing-separator: true normalize-field-content: true max-line-length: 79 - normalize-stanza-order: true + # Latest evaluation on normalize-stanza order + # -> https://salsa.debian.org/debian/debputy/-/issues/91#note_492437 + normalize-stanza-order: false # Not yet implemented: # normalize-field-order: true diff --git a/tests/lint_tests/test_lint_changelog.py b/tests/lint_tests/test_lint_changelog.py index 258c2fe..b5ab69e 100644 --- a/tests/lint_tests/test_lint_changelog.py +++ b/tests/lint_tests/test_lint_changelog.py @@ -31,13 +31,13 @@ def test_dctrl_lint(line_linter: LintWrapper) -> None: """\ foo (0.2) unstable; urgency=medium - * Renamed to foo + * Renamed to foo -- Niels Thykier <niels@thykier.net> Mon, 08 Apr 2024 16:00:00 +0000 bar (0.2) unstable; urgency=medium - * Initial release + * Initial release -- Niels Thykier <niels@thykier.net> Mon, 01 Apr 2024 00:00:00 +0000 """ @@ -82,3 +82,38 @@ def test_dctrl_lint(line_linter: LintWrapper) -> None: assert diag.severity == DiagnosticSeverity.Error assert diag.message == msg assert f"{diag.range}" == "0:0-0:3" + + +def test_dctrl_lint_historical(line_linter: LintWrapper) -> None: + nonsense = "very very very very very very very very very very very very very very " + lines = textwrap.dedent( + f"""\ + foo (0.4) unstable; urgency=medium + + * A {nonsense} long line about absolute nothing that should trigger a warning about length. + + -- Niels Thykier <niels@thykier.net> Mon, 08 Apr 2024 16:00:00 +0000 + + foo (0.3) unstable; urgency=medium + + * Another entry that is not too long. + + -- Niels Thykier <niels@thykier.net> Thu, 04 Apr 2024 00:00:00 +0000 + + foo (0.2) unstable; urgency=medium + + * A {nonsense} long line about absolute nothing that should not trigger a warning about length. + + -- Niels Thykier <niels@thykier.net> Mon, 01 Apr 2024 00:00:00 +0000 + """ + ).splitlines(keepends=True) + diagnostics = line_linter(lines) + print(diagnostics) + # This should be problematic though + assert diagnostics and len(diagnostics) == 1 + diag = diagnostics[0] + + msg = "Line exceeds 82 characters" + assert diag.severity == DiagnosticSeverity.Hint + assert diag.message == msg + assert f"{diag.range}" == "2:82-2:153" diff --git a/tests/lint_tests/test_lint_dctrl.py b/tests/lint_tests/test_lint_dctrl.py index 7e9477e..c91d43d 100644 --- a/tests/lint_tests/test_lint_dctrl.py +++ b/tests/lint_tests/test_lint_dctrl.py @@ -470,6 +470,33 @@ def test_dctrl_lint_sv(line_linter: LintWrapper) -> None: assert issue.severity == DiagnosticSeverity.Information +def test_dctrl_lint_sv_udeb_only(line_linter: LintWrapper) -> None: + lines = textwrap.dedent( + f"""\ + Source: foo + Section: devel + Priority: optional + Maintainer: Jane Developer <jane@example.com> + Build-Depends: debhelper-compat (= 13) + + Package: foo-udeb + Architecture: all + Package-Type: udeb + Section: debian-installer + Depends: bar, baz + Description: Some very interesting synopsis + A very interesting description + that spans multiple lines + . + Just so be clear, this is for a test. + """ + ).splitlines(keepends=True) + + diagnostics = line_linter(lines) + print(diagnostics) + assert not diagnostics + + def test_dctrl_lint_multiple_vcs(line_linter: LintWrapper) -> None: lines = textwrap.dedent( f"""\ @@ -508,3 +535,136 @@ def test_dctrl_lint_multiple_vcs(line_linter: LintWrapper) -> None: assert second_issue.message == msg assert f"{second_issue.range}" == "7:0-8:0" assert second_issue.severity == DiagnosticSeverity.Warning + + +def test_dctrl_lint_synopsis_empty(line_linter: LintWrapper) -> None: + lines = textwrap.dedent( + f"""\ + Source: foo + Section: devel + Priority: optional + Standards-Version: {CURRENT_STANDARDS_VERSION} + Maintainer: Jane Developer <jane@example.com> + Build-Depends: debhelper-compat (= 13) + + Package: foo + Architecture: all + Depends: bar, baz + Description: + A very interesting description + without a synopsis + . + Just so be clear, this is for a test. + """ + ).splitlines(keepends=True) + + diagnostics = line_linter(lines) + print(diagnostics) + assert diagnostics and len(diagnostics) == 1 + issue = diagnostics[0] + + msg = "Package synopsis is missing" + assert issue.message == msg + assert f"{issue.range}" == "10:0-10:11" + assert issue.severity == DiagnosticSeverity.Warning + + +def test_dctrl_lint_synopsis_basis(line_linter: LintWrapper) -> None: + lines = textwrap.dedent( + f"""\ + Source: foo + Section: devel + Priority: optional + Standards-Version: {CURRENT_STANDARDS_VERSION} + Maintainer: Jane Developer <jane@example.com> + Build-Depends: debhelper-compat (= 13) + + Package: foo + Architecture: all + Depends: bar, baz + Description: The synopsis is not the best because it starts with an article and also the synopsis goes on and on + A very interesting description + with a poor synopsis + . + Just so be clear, this is for a test. + """ + ).splitlines(keepends=True) + + diagnostics = line_linter(lines) + print(diagnostics) + assert diagnostics and len(diagnostics) == 2 + first_issue, second_issue = diagnostics + + msg = "Package synopsis starts with an article (a/an/the)." + assert first_issue.message == msg + assert f"{first_issue.range}" == "10:13-10:16" + assert first_issue.severity == DiagnosticSeverity.Warning + + msg = "Package synopsis is too long." + assert second_issue.message == msg + assert f"{second_issue.range}" == "10:92-10:112" + assert second_issue.severity == DiagnosticSeverity.Warning + + +def test_dctrl_lint_synopsis_template(line_linter: LintWrapper) -> None: + lines = textwrap.dedent( + f"""\ + Source: foo + Section: devel + Priority: optional + Standards-Version: {CURRENT_STANDARDS_VERSION} + Maintainer: Jane Developer <jane@example.com> + Build-Depends: debhelper-compat (= 13) + + Package: foo + Architecture: all + Depends: bar, baz + Description: <insert up to 60 chars description> + A very interesting description + with a poor synopsis + . + Just so be clear, this is for a test. + """ + ).splitlines(keepends=True) + + diagnostics = line_linter(lines) + print(diagnostics) + assert diagnostics and len(diagnostics) == 1 + issue = diagnostics[0] + + msg = "Package synopsis is a placeholder" + assert issue.message == msg + assert f"{issue.range}" == "10:13-10:48" + assert issue.severity == DiagnosticSeverity.Warning + + +def test_dctrl_lint_synopsis_too_short(line_linter: LintWrapper) -> None: + lines = textwrap.dedent( + f"""\ + Source: foo + Section: devel + Priority: optional + Standards-Version: {CURRENT_STANDARDS_VERSION} + Maintainer: Jane Developer <jane@example.com> + Build-Depends: debhelper-compat (= 13) + + Package: foo + Architecture: all + Depends: bar, baz + Description: short + A very interesting description + with a poor synopsis + . + Just so be clear, this is for a test. + """ + ).splitlines(keepends=True) + + diagnostics = line_linter(lines) + print(diagnostics) + assert diagnostics and len(diagnostics) == 1 + issue = diagnostics[0] + + msg = "Package synopsis is too short" + assert issue.message == msg + assert f"{issue.range}" == "10:13-10:18" + assert issue.severity == DiagnosticSeverity.Warning diff --git a/tests/lint_tests/test_lint_dtctrl.py b/tests/lint_tests/test_lint_dtctrl.py new file mode 100644 index 0000000..6127dd9 --- /dev/null +++ b/tests/lint_tests/test_lint_dtctrl.py @@ -0,0 +1,74 @@ +import textwrap + +import pytest + +from debputy.lsp.lsp_debian_control_reference_data import CURRENT_STANDARDS_VERSION +from debputy.lsp.lsp_debian_tests_control import _lint_debian_tests_control +from debputy.packages import DctrlParser +from debputy.plugin.api.feature_set import PluginProvidedFeatureSet +from lint_tests.lint_tutil import ( + group_diagnostics_by_severity, + requires_levenshtein, + LintWrapper, +) + +try: + from lsprotocol.types import Diagnostic, DiagnosticSeverity +except ImportError: + pass + + +@pytest.fixture +def line_linter( + debputy_plugin_feature_set: PluginProvidedFeatureSet, + lint_dctrl_parser: DctrlParser, +) -> LintWrapper: + return LintWrapper( + "/nowhere/debian/tests/control", + _lint_debian_tests_control, + debputy_plugin_feature_set, + lint_dctrl_parser, + ) + + +@requires_levenshtein +def test_dtctrl_lint_live_example_silx(line_linter: LintWrapper) -> None: + lines = textwrap.dedent( + """\ + Tests: no-opencl + Depends: + @, + python3-all, + python3-pytest, + python3-pytest-mock, + python3-pytest-xvfb, + xauth, + xvfb, + Restrictions: allow-stderr + + Tests: opencl + Depends: + @, + clinfo, + python3-all, + python3-pytest, + python3-pytest-mock, + python3-pytest-xvfb, + xauth, + xvfb, + Architecture: !i386 + Restrictions: allow-stderr + + Test-Command: xvfb-run -s "-screen 0 1024x768x24 -ac +extension GLX +render -noreset" sh debian/tests/gui + Depends: + mesa-utils, + silx, + xauth, + xvfb, + Restrictions: allow-stderr + """ + ).splitlines(keepends=True) + + diagnostics = line_linter(lines) + print(diagnostics) + assert not diagnostics diff --git a/tests/test_style.py b/tests/test_style.py index 0e3f5d7..ef6ddc4 100644 --- a/tests/test_style.py +++ b/tests/test_style.py @@ -25,7 +25,7 @@ def test_load_styles() -> None: assert nt_style.formatting_deb822_always_wrap assert nt_style.formatting_deb822_trailing_separator assert nt_style.formatting_deb822_max_line_length == 79 - assert nt_style.formatting_deb822_normalize_stanza_order + assert not nt_style.formatting_deb822_normalize_stanza_order # TODO: Not implemented yet assert not nt_style.formatting_deb822_normalize_field_order @@ -41,7 +41,7 @@ def test_load_named_styles() -> None: assert black_style.formatting_deb822_always_wrap assert black_style.formatting_deb822_trailing_separator assert black_style.formatting_deb822_max_line_length == 79 - assert black_style.formatting_deb822_normalize_stanza_order + assert not black_style.formatting_deb822_normalize_stanza_order # TODO: Not implemented yet assert not black_style.formatting_deb822_normalize_field_order |