diff options
Diffstat (limited to 'src')
-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 |
12 files changed, 772 insertions, 259 deletions
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 |