diff options
| -rw-r--r-- | debputy/plugins/debputy-documentation.json | 7 | ||||
| -rw-r--r-- | src/debputy/analysis/debian_dir.py | 33 | ||||
| -rw-r--r-- | src/debputy/dh/dh_assistant.py | 23 | ||||
| -rw-r--r-- | src/debputy/linting/lint_impl.py | 2 | ||||
| -rw-r--r-- | src/debputy/lsp/lsp_debian_control.py | 130 | ||||
| -rw-r--r-- | src/debputy/packager_provided_files.py | 28 | ||||
| -rw-r--r-- | tests/lint_tests/test_lint_dctrl.py | 131 |
7 files changed, 295 insertions, 59 deletions
diff --git a/debputy/plugins/debputy-documentation.json b/debputy/plugins/debputy-documentation.json index 4017e2c..2cfde53 100644 --- a/debputy/plugins/debputy-documentation.json +++ b/debputy/plugins/debputy-documentation.json @@ -46,6 +46,13 @@ "file-categories": ["maint-config"] }, { + "path": "debian/lintian-brush.conf", + "documentation-uris": [ + "man:lintian-brush.conf(5)" + ], + "file-categories": ["maint-config"] + }, + { "path": "debian/source/options", "documentation-uris": [ "man:dpkg-source(1)" diff --git a/src/debputy/analysis/debian_dir.py b/src/debputy/analysis/debian_dir.py index 1e88b14..8ada2a0 100644 --- a/src/debputy/analysis/debian_dir.py +++ b/src/debputy/analysis/debian_dir.py @@ -16,6 +16,7 @@ from typing import ( Iterator, TypedDict, NotRequired, + Container, ) from debputy.analysis import REFERENCE_DATA_TABLE @@ -23,6 +24,7 @@ from debputy.analysis.analysis_util import flatten_ppfs from debputy.dh.dh_assistant import ( resolve_active_and_inactive_dh_commands, read_dh_addon_sequences, + extract_dh_compat_level, ) from debputy.packager_provided_files import ( PackagerProvidedFile, @@ -105,7 +107,7 @@ def scan_debian_dir( or "zz_debputy" in dh_sequences or "zz-debputy-rrr" in dh_sequences ) - dh_compat_level, dh_assistant_exit_code = _extract_dh_compat_level() + dh_compat_level, dh_assistant_exit_code = extract_dh_compat_level() dh_issues = [] static_packaging_files = { @@ -128,6 +130,7 @@ def scan_debian_dir( binary_packages, allow_fuzzy_matches=True, detect_typos=True, + ignore_paths=static_packaging_files, ) ) ) @@ -139,7 +142,7 @@ def scan_debian_dir( all_dh_ppfs, dh_issues, dh_assistant_exit_code, - ) = _resolve_debhelper_config_files( + ) = resolve_debhelper_config_files( debian_dir, binary_packages, debputy_plugin_metadata, @@ -147,6 +150,7 @@ def scan_debian_dir( dh_sequences, dh_compat_level, uses_dh_sequencer, + ignore_paths=static_packaging_files, ) else: @@ -285,7 +289,7 @@ def _kpf_install_pattern( return ppkpf.info.get("install_pattern") -def _resolve_debhelper_config_files( +def resolve_debhelper_config_files( debian_dir: VirtualPath, binary_packages: Mapping[str, BinaryPackage], debputy_plugin_metadata: DebputyPluginMetadata, @@ -293,6 +297,7 @@ def _resolve_debhelper_config_files( dh_rules_addons: AbstractSet[str], dh_compat_level: int, saw_dh: bool, + ignore_paths: Container[str] = frozenset(), ) -> Tuple[List[PackagerProvidedFile], Optional[object], int]: dh_ppfs = {} commands, exit_code = _relevant_dh_commands(dh_rules_addons) @@ -434,6 +439,7 @@ def _resolve_debhelper_config_files( binary_packages, allow_fuzzy_matches=True, detect_typos=True, + ignore_paths=ignore_paths, ) ) ) @@ -532,27 +538,6 @@ def _merge_ppfs( _merge_list(details, "documentation-uris", documentation_uris) -def _extract_dh_compat_level() -> Tuple[Optional[int], int]: - try: - output = subprocess.check_output( - ["dh_assistant", "active-compat-level"], - stderr=subprocess.DEVNULL, - ) - except (FileNotFoundError, subprocess.CalledProcessError) as e: - exit_code = 127 - if isinstance(e, subprocess.CalledProcessError): - exit_code = e.returncode - return None, exit_code - else: - data = json.loads(output) - active_compat_level = data.get("active-compat-level") - exit_code = 0 - if not isinstance(active_compat_level, int) or active_compat_level < 1: - active_compat_level = None - exit_code = 255 - return active_compat_level, exit_code - - def _relevant_dh_commands(dh_rules_addons: Iterable[str]) -> Tuple[List[str], int]: cmd = ["dh_assistant", "list-commands", "--output-format=json"] if dh_rules_addons: diff --git a/src/debputy/dh/dh_assistant.py b/src/debputy/dh/dh_assistant.py index ba8c14f..836b1b5 100644 --- a/src/debputy/dh/dh_assistant.py +++ b/src/debputy/dh/dh_assistant.py @@ -7,7 +7,6 @@ from typing import Iterable, FrozenSet, Optional, List, Union, Mapping, Any, Set from debian.deb822 import Deb822 from debputy.plugin.api import VirtualPath -from debputy.util import _info _FIND_DH_WITH = re.compile(r"--with(?:\s+|=)(\S+)") _DEP_REGEX = re.compile("^([a-z0-9][-+.a-z0-9]+)", re.ASCII) @@ -123,3 +122,25 @@ def read_dh_addon_sequences( extract_dh_addons_from_control(source_paragraph, bd_sequences) return bd_sequences, dr_sequences, saw_dh return None + + +def extract_dh_compat_level(*, cwd=None) -> Tuple[Optional[int], int]: + try: + output = subprocess.check_output( + ["dh_assistant", "active-compat-level"], + stderr=subprocess.DEVNULL, + cwd=cwd, + ) + except (FileNotFoundError, subprocess.CalledProcessError) as e: + exit_code = 127 + if isinstance(e, subprocess.CalledProcessError): + exit_code = e.returncode + return None, exit_code + else: + data = json.loads(output) + active_compat_level = data.get("active-compat-level") + exit_code = 0 + if not isinstance(active_compat_level, int) or active_compat_level < 1: + active_compat_level = None + exit_code = 255 + return active_compat_level, exit_code diff --git a/src/debputy/linting/lint_impl.py b/src/debputy/linting/lint_impl.py index b424460..ddafd4c 100644 --- a/src/debputy/linting/lint_impl.py +++ b/src/debputy/linting/lint_impl.py @@ -276,7 +276,7 @@ def perform_reformat( https://salsa.debian.org/debian/debputy-pre-commit-hooks * If you use the Debian Salsa CI pipeline, then you can set SALSA_CI_DISABLE_WRAP_AND_SORT - to a truth value and `debputy` will pick up the configuration from there. + to a "no" or 0 and `debputy` will pick up the configuration from there. - Note: The option must be in `.gitlab-ci.yml` or `debian/salsa-ci.yml` to work. The Salsa CI pipeline will use `wrap-and-sort` while `debputy` uses its own emulation of `wrap-and-sort` (`debputy` also needs to apply the style via `debputy lsp server`). diff --git a/src/debputy/lsp/lsp_debian_control.py b/src/debputy/lsp/lsp_debian_control.py index 5bb6265..05cd943 100644 --- a/src/debputy/lsp/lsp_debian_control.py +++ b/src/debputy/lsp/lsp_debian_control.py @@ -11,9 +11,12 @@ from typing import ( List, Dict, Iterable, + Container, ) -from debputy.analysis.debian_dir import scan_debian_dir +from debputy.analysis.analysis_util import flatten_ppfs +from debputy.analysis.debian_dir import scan_debian_dir, resolve_debhelper_config_files +from debputy.dh.dh_assistant import extract_dh_compat_level from debputy.linting.lint_util import LintState from debputy.lsp.debputy_ls import DebputyLanguageServer from debputy.lsp.diagnostics import DiagnosticData @@ -92,6 +95,12 @@ from debputy.lsprotocol.types import ( InlayHint, InlayHintLabelPart, ) +from debputy.packager_provided_files import ( + PackagerProvidedFile, + detect_all_packager_provided_files, +) +from debputy.plugin.api import VirtualPath +from debputy.plugin.api.impl import plugin_metadata_for_debputys_own_plugin from debputy.util import detect_possible_typo try: @@ -1042,43 +1051,108 @@ def _package_range_of_stanza( yield stanza["Package"], stanza.get("Architecture"), representation_field_range -def _detect_misspelled_packaging_files( +def _packaging_files( lint_state: LintState, - binary_stanzas_w_pos: List[Tuple[Deb822ParagraphElement, TEPosition]], - diagnostics: List[Diagnostic], -) -> None: +) -> Iterable[PackagerProvidedFile]: + source_root = lint_state.source_root debian_dir = lint_state.debian_dir binary_packages = lint_state.binary_packages - if debian_dir is None or binary_packages is None: + if ( + source_root is None + or not source_root.has_fs_path + or debian_dir is None + or binary_packages is None + ): return dh_sequencer_data = lint_state.dh_sequencer_data - - all_pkg_file_data, _, _, _ = scan_debian_dir( - lint_state.plugin_feature_set, - binary_packages, - debian_dir, - uses_dh_sequencer=dh_sequencer_data.uses_dh_sequencer, - dh_sequences=dh_sequencer_data.sequences, + dh_sequences = dh_sequencer_data.sequences + is_debputy_package = ( + "debputy" in dh_sequences + or "zz-debputy" in dh_sequences + or "zz_debputy" in dh_sequences + or "zz-debputy-rrr" in dh_sequences ) + feature_set = lint_state.plugin_feature_set + known_packaging_files = feature_set.known_packaging_files + static_packaging_files = { + kpf.detection_value: kpf + for kpf in known_packaging_files.values() + if kpf.detection_method == "path" + } + ignored_path = set(static_packaging_files) + + if is_debputy_package: + all_debputy_ppfs = list( + flatten_ppfs( + detect_all_packager_provided_files( + feature_set.packager_provided_files, + debian_dir, + binary_packages, + allow_fuzzy_matches=True, + detect_typos=True, + ignore_paths=ignored_path, + ) + ) + ) + for ppf in all_debputy_ppfs: + if ppf.path.path in ignored_path: + continue + ignored_path.add(ppf.path.path) + yield ppf + + # FIXME: This should read the editor data, but dh_assistant does not support that. + dh_compat_level, _ = extract_dh_compat_level(cwd=source_root.fs_path) + if dh_compat_level is not None: + debputy_plugin_metadata = plugin_metadata_for_debputys_own_plugin() + dh_pkgfile_docs = { + kpf.detection_value: kpf + for kpf in known_packaging_files.values() + if kpf.detection_method == "dh.pkgfile" + } + ( + all_dh_ppfs, + _, + _, + ) = resolve_debhelper_config_files( + debian_dir, + binary_packages, + debputy_plugin_metadata, + dh_pkgfile_docs, + dh_sequences, + dh_compat_level, + saw_dh=dh_sequencer_data.uses_dh_sequencer, + ignore_paths=ignored_path, + ) + for ppf in all_dh_ppfs: + if ppf.path.path in ignored_path: + continue + ignored_path.add(ppf.path.path) + yield ppf + + +def _detect_misspelled_packaging_files( + lint_state: LintState, + binary_stanzas_w_pos: List[Tuple[Deb822ParagraphElement, TEPosition]], + diagnostics: List[Diagnostic], +) -> None: stanza_ranges = { p: (a, r) for p, a, r in _package_range_of_stanza(lint_state, binary_stanzas_w_pos) } - - for pkg_file_data in all_pkg_file_data: - binary_package = pkg_file_data.get("binary-package") - explicit_package = pkg_file_data.get("pkgfile-explicit-package-name", True) - name_segment = pkg_file_data.get("pkgfile-name-segment") - stem = pkg_file_data.get("pkgfile-stem") + for ppf in _packaging_files(lint_state): + binary_package = ppf.package_name + explicit_package = ppf.uses_explicit_package_name + name_segment = ppf.name_segment is not None + stem = ppf.definition.stem if binary_package is None or stem is None: continue declared_arch, diag_range = stanza_ranges.get(binary_package) if diag_range is None: continue - path = pkg_file_data["path"] - likely_typo_of = pkg_file_data.get("likely-typo-of") - arch_restriction = pkg_file_data.get("pkgfile-architecture-restriction") + path = ppf.path.path + likely_typo_of = ppf.expected_path + arch_restriction = ppf.architecture_restriction if likely_typo_of is not None: # Handles arch_restriction == 'all' at the same time due to how # the `likely-typo-of` is created @@ -1120,7 +1194,7 @@ def _detect_misspelled_packaging_files( ) ) - if not pkg_file_data.get("pkgfile-is-active-in-build", True): + if not ppf.definition.has_active_command: diagnostics.append( Diagnostic( diag_range, @@ -1137,14 +1211,20 @@ def _detect_misspelled_packaging_files( if not explicit_package and name_segment is not None: basename = os.path.basename(path) + if basename == ppf.definition.stem: + continue alt_name = f"{binary_package}.{stem}" if arch_restriction is not None: alt_name = f"{alt_name}.{arch_restriction}" + if ppf.definition.allow_name_segment: + or_alt_name = f' (or maybe "debian/{binary_package}.{basename}")' + else: + or_alt_name = "" diagnostics.append( Diagnostic( diag_range, - f'Possible typo in "{path}". Consider renaming the file to "debian/{binary_package}.{basename}"' - f' or "debian/{alt_name} if it is intended for {binary_package}', + f'Possible typo in "{path}". Consider renaming the file to "debian/{alt_name}"' + f"{or_alt_name} if it is intended for {binary_package}", severity=DiagnosticSeverity.Warning, source="debputy", data=DiagnosticData( diff --git a/src/debputy/packager_provided_files.py b/src/debputy/packager_provided_files.py index 8e0e0af..d2512f4 100644 --- a/src/debputy/packager_provided_files.py +++ b/src/debputy/packager_provided_files.py @@ -1,6 +1,6 @@ import collections import dataclasses -from typing import Mapping, Iterable, Dict, List, Optional, Tuple, Sequence +from typing import Mapping, Iterable, Dict, List, Optional, Tuple, Sequence, Container from debputy.packages import BinaryPackage from debputy.plugin.api import VirtualPath @@ -10,7 +10,11 @@ from debputy.util import _error, CAN_DETECT_TYPOS, detect_possible_typo _KNOWN_NON_PPFS = frozenset( { + # Some of these overlap with the _KNOWN_NON_TYPO_EXTENSIONS below + # This one is a quicker check. The _KNOWN_NON_TYPO_EXTENSIONS is a general (but more + # expensive check). "gbp.conf", # Typo matches with `gbp.config` (dh_installdebconf) in two edits steps + "salsa-ci.yml", # Typo matches with `salsa-ci.wm` (dh_installwm) in two edits steps # No reason to check any of these as they are never PPFs "clean", "control", @@ -21,6 +25,19 @@ _KNOWN_NON_PPFS = frozenset( } ) +_KNOWN_NON_TYPO_EXTENSIONS = frozenset( + { + "conf", + "sh", + "yml", + "yaml", + "json", + "bash", + "pl", + "py", + } +) + @dataclasses.dataclass(frozen=True, slots=True) class PackagerProvidedFile: @@ -107,6 +124,7 @@ def _find_definition( basename: str, *, period2stems: Optional[Mapping[int, Sequence[str]]] = None, + had_arch: bool = False, ) -> Tuple[Optional[str], Optional[PackagerProvidedFileClassSpec], Optional[str]]: for stem, install_as_name, period_count in _iterate_stem_splits(basename): definition = packager_provided_files.get(stem) @@ -118,6 +136,10 @@ def _find_definition( if not stems: continue + # If the stem is also the extension and a known one at that, then + # we do not consider it a typo match (to avoid false positives). + if not had_arch and stem in _KNOWN_NON_TYPO_EXTENSIONS: + continue matches = detect_possible_typo(stem, stems) if matches is not None and len(matches) == 1: definition = packager_provided_files[matches[0]] @@ -252,6 +274,7 @@ def _split_path( packager_provided_files, basename, period2stems=period2stems, + had_arch=bool(arch_restriction), ) if definition is None: continue @@ -365,6 +388,7 @@ def detect_all_packager_provided_files( *, allow_fuzzy_matches: bool = False, detect_typos: bool = False, + ignore_paths: Container[str] = frozenset(), ) -> Dict[str, PerPackagePackagerProvidedResult]: main_binary_package = [ p.name for p in binary_packages.values() if p.is_main_package @@ -381,7 +405,7 @@ def detect_all_packager_provided_files( for entry in debian_dir.iterdir: if entry.is_dir: continue - if entry.name in _KNOWN_NON_PPFS: + if entry.path in ignore_paths or entry.name in _KNOWN_NON_PPFS: continue matching_ppfs = _split_path( packager_provided_files, diff --git a/tests/lint_tests/test_lint_dctrl.py b/tests/lint_tests/test_lint_dctrl.py index 745c323..840fabe 100644 --- a/tests/lint_tests/test_lint_dctrl.py +++ b/tests/lint_tests/test_lint_dctrl.py @@ -732,7 +732,12 @@ def test_dctrl_lint_ambiguous_pkgfile(line_linter: LintWrapper) -> None: # FIXME: This relies on "cwd" being a valid debian directory using debhelper. Fix and # remove the `build_time_only` restriction - line_linter.source_root = build_virtual_file_system(["./debian/bar.install"]) + line_linter.source_root = build_virtual_file_system( + [ + virtual_path_def(".", fs_path="."), + "./debian/bar.service", + ] + ) diagnostics = line_linter(lines) print(diagnostics) @@ -740,8 +745,8 @@ def test_dctrl_lint_ambiguous_pkgfile(line_linter: LintWrapper) -> None: issue = diagnostics[0] msg = ( - 'Possible typo in "./debian/bar.install". Consider renaming the file to "debian/foo.bar.install"' - ' or "debian/foo.install if it is intended for foo' + 'Possible typo in "./debian/bar.service". Consider renaming the file to "debian/foo.service"' + ' (or maybe "debian/foo.bar.service") if it is intended for foo' ) assert issue.message == msg assert f"{issue.range}" == "7:0-8:0" @@ -749,8 +754,59 @@ def test_dctrl_lint_ambiguous_pkgfile(line_linter: LintWrapper) -> None: diag_data = issue.data assert isinstance(diag_data, dict) assert diag_data.get("report_for_related_file") in ( - "./debian/bar.install", - "debian/bar.install", + "./debian/bar.service", + "debian/bar.service", + ) + + +@build_time_only +def test_dctrl_lint_ambiguous_pkgfile_no_name_segment(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), dh-sequence-zz-debputy, + + Package: foo + Architecture: all + Depends: bar, baz + Description: some short synopsis + A very interesting description + with a valid synopsis + . + Just so be clear, this is for a test. + """ + ).splitlines(keepends=True) + + # FIXME: This relies on "cwd" being a valid debian directory using debhelper. Fix and + # remove the `build_time_only` restriction + line_linter.source_root = build_virtual_file_system( + [ + virtual_path_def(".", fs_path="."), + "./debian/bar.alternatives", + ] + ) + + diagnostics = line_linter(lines) + print(diagnostics) + assert diagnostics and len(diagnostics) == 1 + issue = diagnostics[0] + + msg = ( + 'Possible typo in "./debian/bar.alternatives". Consider renaming the file to "debian/foo.alternatives"' + " if it is intended for foo" + ) + assert issue.message == msg + assert f"{issue.range}" == "7:0-8:0" + assert issue.severity == DiagnosticSeverity.Warning + diag_data = issue.data + assert isinstance(diag_data, dict) + assert diag_data.get("report_for_related_file") in ( + "./debian/bar.alternatives", + "debian/bar.alternatives", ) @@ -779,7 +835,12 @@ def test_dctrl_lint_stem_typo_pkgfile(line_linter: LintWrapper) -> None: # FIXME: This relies on "cwd" being a valid debian directory using debhelper. Fix and # remove the `build_time_only` restriction - line_linter.source_root = build_virtual_file_system(["./debian/foo.intsall"]) + line_linter.source_root = build_virtual_file_system( + [ + virtual_path_def(".", fs_path="."), + "./debian/foo.intsall", + ] + ) diagnostics = line_linter(lines) print(diagnostics) @@ -827,6 +888,7 @@ def test_dctrl_lint_stem_inactive_pkgfile_fp(line_linter: LintWrapper) -> None: # load the `zz-debputy` sequence. line_linter.source_root = build_virtual_file_system( [ + virtual_path_def(".", fs_path="."), "./debian/foo.install", virtual_path_def( "./debian/rules", @@ -846,3 +908,60 @@ def test_dctrl_lint_stem_inactive_pkgfile_fp(line_linter: LintWrapper) -> None: print(diagnostics) # We should not emit diagnostics when the package is not using dh! assert not diagnostics + + +@requires_levenshtein +@build_time_only +def test_dctrl_lint_stem_typo_pkgfile_ignored_exts_or_files( + 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: some short synopsis + A very interesting description + with a valid synopsis + . + Just so be clear, this is for a test. + """ + ).splitlines(keepends=True) + + # FIXME: This relies on "cwd" being a valid debian directory using debhelper. Fix and + # remove the `build_time_only` restriction + line_linter.source_root = build_virtual_file_system( + [ + virtual_path_def(".", fs_path="."), + "debian/salsa-ci.yml", + "debian/gbp.conf", + "debian/foo.conf", + "debian/foo.sh", + "debian/foo.yml", + # One wrong one to ensure the test works. + "debian/foo.intsall", + ] + ) + + diagnostics = line_linter(lines) + print(diagnostics) + assert diagnostics and len(diagnostics) == 1 + issue = diagnostics[0] + + msg = 'The file "./debian/foo.intsall" is likely a typo of "./debian/foo.install"' + assert issue.message == msg + assert f"{issue.range}" == "7:0-8:0" + assert issue.severity == DiagnosticSeverity.Warning + diag_data = issue.data + assert isinstance(diag_data, dict) + assert diag_data.get("report_for_related_file") in ( + "./debian/foo.intsall", + "debian/foo.intsall", + ) |