diff options
32 files changed, 750 insertions, 139 deletions
diff --git a/GETTING-STARTED-WITH-dh-debputy.md b/GETTING-STARTED-WITH-dh-debputy.md index ac0b034..7b6796b 100644 --- a/GETTING-STARTED-WITH-dh-debputy.md +++ b/GETTING-STARTED-WITH-dh-debputy.md @@ -53,7 +53,7 @@ Note: More options may appear in the future. ## Step 2: Have `debputy` convert relevant `debhelper` files The `debputy` integration with `debhelper` removes (replaces) some debhelper tools, but does **not** -read their relevant config files. These should instead be converted in to the new format. +read their relevant config files. These should instead be converted in to the new format. You can have `debputy` convert these for you by using the following command: @@ -247,14 +247,14 @@ using complex rules for pruning or moving around files in a way that are not eas _This is section does **not** apply to the `dh-sequence-zz-debputy-rrr` integration mode._ -The `dh_installman` tool auto-detects the language for manpages via two different methods: +The `dh_installman` tool auto-detects the language for man pages via two different methods: 1) By path name (Does the installation path look like `man/<language>/man<section>/...`?) 2) By basename (Does the basename look like `foo.<language>.<section>`?) Both methods are used in order with first match being the method of choice. Unfortunately, the second method is prune to false-positives. Does `foo.pl.1` mean a Polish translation of `foo.1` or is it the -manpage for a Perl script called `foo.pl` (similar happens for other languages/file extensions). +man page for a Perl script called `foo.pl` (similar happens for other languages/file extensions). To avoid this issue, `debputy` uses 1) by default and only that. Option 2) can be chosen by setting `language: derive-from-basename` on the concrete installation rule. The problem is that the migration tool @@ -312,7 +312,7 @@ _Remember to merge your manifest with previous steps rather than replacing it!_ `debputy migrate-from-dh` will merge its changes into existing manifests and can safely be re-run after adding/writing this base manifest. -### Covert your overrides for `dh_installsystemd`, `dh_installinit` (if any) +### Convert your overrides for `dh_installsystemd`, `dh_installinit` (if any) If your package overrides any of the service related helpers, the following use-cases have a trivial known solution: diff --git a/MANIFEST-FORMAT.md b/MANIFEST-FORMAT.md index 08064f1..d1474bf 100644 --- a/MANIFEST-FORMAT.md +++ b/MANIFEST-FORMAT.md @@ -38,11 +38,11 @@ rely entirely on the built-in rules. # Path matching rules Most of the manifest is about declaring rules for a given path such as "foo must be a symlink" -or "bar must be owned by root:tty and have mode 02755". +or "bar must be owned by root:tty and have mode 02755". The manifest contains the following types of matches: - 1) Exact path matches. These specify the path inside the debian package exactly without any + 1) Exact path matches. These specify the path inside the Debian package exactly without any form of wildcards (e.g., `*` or `?`). However, they can use substitution variables. Examples include: * `usr/bin/sudo` @@ -97,7 +97,7 @@ These limitations are in place because of implementation details in `debputy`. The manifest can contain seemly mutually exclusive rules. As an example, if you ask for `foo/symlink` to be a symlink but also state that you want to remove `foo` entirely -from the debian package then the manifest now has two mutually exclusive requests. +from the Debian package then the manifest now has two mutually exclusive requests. To resolve these problems, `debputy` relies on the following rules for conflict resolutions: @@ -151,7 +151,7 @@ In general, the following rules applies to globs in `debputy`. * The `*` match 0 or more times any characters except `/`. * The `?` match exactly one character except `/`. - * The glob `foo/*` matches _everything_ inside `foo/` including hidden files (i.e., paths starting + * The glob `foo/*` matches _everything_ inside `foo/` including hidden files (i.e., paths starting with `.`) unlike `bash`/`sh` globs. However, `foo/*` does _not_ match `foo/` itself (this latter part matches `bash`/`sh` behaviour and should be unsurprising). * For the special-cases where `**` is supported, then `**` matches zero or more levels of directories. @@ -166,7 +166,7 @@ work. The rules will explicitly list if they divert from the above listed glob r The `debputy` tool supports substitution in various places (usually paths) via the following rules. That means: - 1) All substitutions must start with `{{` and end with `}}`. The part inbetween is + 1) All substitutions must start with `{{` and end with `}}`. The part between is the `MANIFEST_VARIABLE` and must match the regular expression `[A-Za-z0-9][-_:0-9A-Za-z]*`. Note that you can use space around the variable name if you feel that increases readability. (That is, `{{ FOO }}` can be used as an alternative to `{{FOO}}`). @@ -224,7 +224,7 @@ one of the following formats: 1) A name (e.g., `owner: root`). 2) An id (e.g., `owner: 0`). Please avoid using quotes around the ID in YAML as that can cause `debputy` to read the number as a name. - 3) A name and an id with a colon inbetween (e.g., `owner: "root:0"`). The name must always + 3) A name and an id with a colon in between (e.g., `owner: "root:0"`). The name must always come first here. You may have to quote the value to prevent the YAML parser from being confused. @@ -243,7 +243,7 @@ Regardless of which form you pick: be invalid as the id for `root` is defined to be `0` in the `base-passwd` data files. 3) The `debputy` tool maintains a `deny`-list of owners that it refuses even though `base-passwd` - defines them. As a notable non-exhaustive example, `debputy` considers `nobody` or id `65534` + defines them. As a notable non-exhaustive example, `debputy` considers `nobody` or id `65534` (the ID of `nobody` / `nogroup`) to be invalid owners. @@ -703,7 +703,7 @@ This install rule resemble that of `dh_installdocs`. It is a shorthand over the With these two things in mind, it behaves just like the `install` rule. Note: It is often worth considering to use a more specialized version of the `install-docs` -rule when one such is available. If you are looking to install an example or a manpage, +rule when one such is available. If you are looking to install an example or a man page, consider whether `install-examples` or `install-man` might be a better fit for your use-case. @@ -805,16 +805,16 @@ or `sources` (respectively). This form can only be used when `into` is not requ Note: While the canonical name for this rule use plural, the `install-example` variant is accepted as alternative name. -## Install manpages (`install-man`) +## Install man pages (`install-man`) -Install rule for installing manpages similar to `dh_installman`. It is a shorthand over the generic +Install rule for installing man pages similar to `dh_installman`. It is a shorthand over the generic `install` rule with the following key features: 1) The rule can only match files (notably, symlinks cannot be matched by this rule). - 2) The `dest-dir` is computed per source file based on the manpage's section and language. + 2) The `dest-dir` is computed per source file based on the man page's section and language. 3) The `into` parameter can be omitted as long as there is a exactly one non-`udeb` package listed in `debian/control`. - 4) The rule comes with manpage specific attributes such as `language` and `section` for when the + 4) The rule comes with man page specific attributes such as `language` and `section` for when the auto-detection is insufficient. 5) The rule comes with pre-defined conditional logic for skipping the rule under `DEB_BUILD_OPTIONS=nodoc`, so you do not have to write that conditional yourself. @@ -841,18 +841,18 @@ has the following key/value pairs: search directories. Only files can be matched. * `into` (conditional): Either a package name or a list of package names for which these paths should be - installed as manpages. This key is conditional on whether there are multiple (non-`udeb`) binary + installed as man pages. This key is conditional on whether there are multiple (non-`udeb`) binary packages listed in `debian/control`. When there is only one (non-`udeb`) binary package, then that binary is the default for `into`. Otherwise, the key is required. * `section` (optional): If provided, it must be an integer between 1 and 9 (both inclusive), defining the - section the manpages belong overriding any auto-detection that `debputy` would have performed. + section the man pages belong overriding any auto-detection that `debputy` would have performed. * `language` (optional): If provided, it must be either a 2 letter language code (such as `de`), a 5 letter language + dialect code (such as `pt_BR`), or one of the special keywords `C`, `derive-from-path`, or `derive-from-basename`. The default is `derive-from-path`. - - When `language` is `C`, then the manpages are assumed to be "untranslated". - - When `language` is a language code (with or without dialect), then all manpages matched will be assumed + - When `language` is `C`, then the man pages are assumed to be "untranslated". + - When `language` is a language code (with or without dialect), then all man pages matched will be assumed to be translated to that concrete language / dialect. - When `language` is `derive-from-path`, then `debputy` attempts to derive the language from the path (`man/<language>/man<section>`). This matches the default of `dh_installman`. When no language can @@ -997,7 +997,7 @@ you might find yourself with some files that are _not_ relevant for the Debian p relevant for other distros or for non-distro local builds). Common examples include `INSTALL` files or `LICENSE` files (when they are just a subset of `debian/copyright`). -In the manifest, you can ask `debputy` to remove paths from the debian package by using the `remove` +In the manifest, you can ask `debputy` to remove paths from the Debian package by using the `remove` transformation rule. An example being: packages: @@ -1361,7 +1361,7 @@ Each mapping has the following key/value pairs: not supported. - `restart`: During an upgrade, `restart` the service post upgrade. The service will be left running during the upgrade process. - - `stop-then-start`: Stop the service before the upgrade, preform the upgrade and then start the service. + - `stop-then-start`: Stop the service before the upgrade, perform the upgrade and then start the service. ### Service managers and aliases @@ -1421,7 +1421,7 @@ Use this feature sparingly as it is generally not possible to undo as each versi higher than the previous one. This feature translates into `-v` option for `dpkg-gencontrol`. The value for the `binary-version` key is a string that defines the binary version. Generally, you will -want it to contain one of the versioned related substitution variables such as +want it to contain one of the versioned related substitution variables such as `{{DEB_VERSION_UPSTREAM_REVISION}}`. Otherwise, you will have to remember to bump the version manually with each upload as versions cannot be reused and the package would not support binNMUs either. diff --git a/MIGRATING-A-DH-PLUGIN.md b/MIGRATING-A-DH-PLUGIN.md index 3e4e227..16184d8 100644 --- a/MIGRATING-A-DH-PLUGIN.md +++ b/MIGRATING-A-DH-PLUGIN.md @@ -518,7 +518,7 @@ Using both options where possible is generally preferable. If your upstream uses a Python test framework that auto-detects tests such as `py.test`, you may find that it picks up the `debputy` plugin or its tests. If this is causing you issues, please have -a look at the `dh_installdebputy` manpage, which have a section dedicated to how to resolve these +a look at the `dh_installdebputy` man page, which have a section dedicated to how to resolve these issues. ## Side note: Python byte-compilation @@ -1,7 +1,7 @@ debputy ======= -The debputy tool is a debian package builder that works with a declarative +The debputy tool is a Debian package builder that works with a declarative manifest format. The early versions will integrate into the debhelper sequencer dh and will replace @@ -71,8 +71,8 @@ The following communication channels are available: * Using the https://salsa.debian.org/debian/debputy features, such as issues. Generally, these will be available to the public. While confidential issues can be filed, please note that - all Debian developers can read it. - * Filing bugs via `reportbug debputy` to the Debian bug tracker. Note all such bugs will have + all Debian Developers can read it. + * Filing bugs via `reportbug debputy` to Debian's Bug Tracking System. Note all such bugs will have public archives. * Mail to Debputy Maintainers <debputy@packages.debian.org>. Please note that no public archive is available but anyone can subscribe via tracker.debian.org. @@ -82,10 +82,10 @@ The following communication channels are available: ## Security issues -For issues that should be embargoed, please report the issue to the Debian Security team, which +For issues that should be embargoed, please report the issue to the Debian Security Team, which has the relevant skill set, policies, and infrastructure to handle this. While the salsa.debian.org services provides support for **confidential** issues, all Debian Developers can read those for the `debputy` project. This makes it unsuitable for security bugs under embargo. -Please review https://www.debian.org/security/faq#contact for how to contact the Debian security -team. +Please review https://www.debian.org/security/faq#contact for how to contact the Debian Security +Team. @@ -14,7 +14,7 @@ people, who supported me early on: <-- -Hi, if you are making a contribution and feel you should be listed here, please +Hi, if you are making a contribution and feel you should be listed here, please uncomment and add yourself below. Additionally, thanks to the following: diff --git a/debputy.pod b/debputy.pod index a88a4f0..6017bf5 100644 --- a/debputy.pod +++ b/debputy.pod @@ -22,7 +22,7 @@ B<debputy> B<lsp> B<server> [B<--tcp|--ws> [B<--host> I<BIND-ADDRESS>] [B<--port =head1 DESCRIPTION -The B<debputy> program is a manifest style Debian-based package builder. This manpage documents some of the +The B<debputy> program is a manifest style Debian-based package builder. This man page documents some of the user facing commands. If you are using B<debputy> with a screen reader, consider setting the environment variable diff --git a/docs/IMPLEMENTATION-DECISIONS.md b/docs/IMPLEMENTATION-DECISIONS.md index a5fad33..d3faef6 100644 --- a/docs/IMPLEMENTATION-DECISIONS.md +++ b/docs/IMPLEMENTATION-DECISIONS.md @@ -196,7 +196,7 @@ integration up and running. When designing it, the following things were import can basically be done as a configuration rather than code. This means that `debputy` can provide automated plugin upgrades from the current format to a future one if needed be. - * Being able to re-use the declarative parser to handle the error messages and data normalization + * Being able to reuse the declarative parser to handle the error messages and data normalization (this implies `JSON`, `YAML` or similar formats that is easily parsed in to mappings and lists). * It is important that there is a plugin API compat level that enables us to change the format or diff --git a/src/debputy/commands/deb_packer.py b/src/debputy/commands/deb_packer.py index 8c61099..986a8fa 100644 --- a/src/debputy/commands/deb_packer.py +++ b/src/debputy/commands/deb_packer.py @@ -288,7 +288,7 @@ def parse_args() -> argparse.Namespace: THIS IS A PROTOTYPE "dpkg-deb -b" emulator with basic manifest support DO NOT USE THIS TOOL DIRECTLY. It has not stability guarantees and will be removed as - soon as "dpkg-deb -b" grows support for the relevant features. + soon as "dpkg-deb -b" grows support for the relevant features. This tool is a prototype "dpkg-deb -b"-like interface for compiling a Debian package without requiring root even for static ownership. It is a temporary stand-in for @@ -297,7 +297,7 @@ def parse_args() -> argparse.Namespace: The tool operates on an internal JSON based manifest for now, because it was faster than building an mtree parser (which is the format that dpkg will likely end up using). - + As the tool is not meant to be used directly, it is full of annoying paper cuts that I refuse to fix or maintain. Use the high level tool instead. diff --git a/src/debputy/commands/debputy_cmd/output.py b/src/debputy/commands/debputy_cmd/output.py index 131338a..df8e6eb 100644 --- a/src/debputy/commands/debputy_cmd/output.py +++ b/src/debputy/commands/debputy_cmd/output.py @@ -243,8 +243,8 @@ class ANSIOutputStylingBase(OutputStylingBase): return super().render_url(link_url) link_text = link_url if not self.optimize_for_screen_reader and link_url.startswith("man:"): - # Rewrite manpage to a clickable link by default. I am not sure how the hyperlink - # ANSI code works with screen readers, so lets not rewrite the manpage link by + # Rewrite man page to a clickable link by default. I am not sure how the hyperlink + # ANSI code works with screen readers, so lets not rewrite the man page link by # default. My fear is that both the link url and the link text gets read out. m = MAN_URL_REWRITE.match(link_url) if m: diff --git a/src/debputy/commands/debputy_cmd/plugin_cmds.py b/src/debputy/commands/debputy_cmd/plugin_cmds.py index 69b2a2a..a8103fb 100644 --- a/src/debputy/commands/debputy_cmd/plugin_cmds.py +++ b/src/debputy/commands/debputy_cmd/plugin_cmds.py @@ -136,7 +136,7 @@ TEXT_CSV_FORMAT_NO_STABILITY_PROMISE = format_output_arg( ) def _plugin_cmd_list_plugins(context: CommandContext) -> None: plugin_metadata_entries = context.load_plugins().plugin_data.values() - # Because the "plugins" part is optional, we are not guaranteed tha TEXT_CSV_FORMAT applies + # Because the "plugins" part is optional, we are not guaranteed that TEXT_CSV_FORMAT applies output_format = getattr(context.parsed_args, "output_format", "text") assert output_format in {"text", "csv"} with _stream_to_pager(context.parsed_args) as (fd, fo): diff --git a/src/debputy/dh_migration/migrators_impl.py b/src/debputy/dh_migration/migrators_impl.py index 450b5a9..d7aa252 100644 --- a/src/debputy/dh_migration/migrators_impl.py +++ b/src/debputy/dh_migration/migrators_impl.py @@ -1079,7 +1079,7 @@ def migrate_installman_file( if warn_about_basename: feature_migration.warn( - 'Detected manpages that might rely on "derive-from-basename" logic. Please double check' + 'Detected man pages that might rely on "derive-from-basename" logic. Please double check' " that the generated `install-man` rules are correct" ) diff --git a/src/debputy/installations.py b/src/debputy/installations.py index 2310cfa..e1e8f3a 100644 --- a/src/debputy/installations.py +++ b/src/debputy/installations.py @@ -212,9 +212,9 @@ def _determine_manpage_real_section( f" which is not a valid section (must be between 1 and 9 incl.)" ) _error( - f"Could not determine the section for {match_rule.path.fs_path} automatically. The manpage" + f"Could not determine the section for {match_rule.path.fs_path} automatically. The man page" f" was detected via {definition_source}. Consider using `section: <number>` to" - " explicitly declare the section. Keep in mind that it applies to all manpages for that" + " explicitly declare the section. Keep in mind that it applies to all man pages for that" " rule and you may have to split the rule into two for this reason." ) return real_section diff --git a/src/debputy/lsp/debian-wordlist.dic b/src/debputy/lsp/debian-wordlist.dic index 11e0438..5d75eaa 100644 --- a/src/debputy/lsp/debian-wordlist.dic +++ b/src/debputy/lsp/debian-wordlist.dic @@ -171,8 +171,6 @@ maintscript maintscripts makefile makefiles -manpage -manpages md5sum md5sums menutest diff --git a/src/debputy/lsp/lsp_debian_control_reference_data.py b/src/debputy/lsp/lsp_debian_control_reference_data.py index 3e16f3c..feed858 100644 --- a/src/debputy/lsp/lsp_debian_control_reference_data.py +++ b/src/debputy/lsp/lsp_debian_control_reference_data.py @@ -233,7 +233,7 @@ def all_architectures_and_wildcards(arch2table) -> Iterable[Union[str, Keyword]] The package is an architecture dependent package and need to be compiled for each and every architecture it. - The name `any` refers to the fact that this is an architecture *wildcard* matching + The name `any` refers to the fact that this is an architecture *wildcard* matching *any machine architecture* supported by dpkg. """ ), @@ -1313,7 +1313,7 @@ BINARY_FIELDS = _fields( "no", hover_text=textwrap.dedent( """\ - The package is a regular package. This is the default and recommended.</p> + The package is a regular package. This is the default and recommended. Note that declaring a package to be "Essential: no" is the same as not having the field except omitting the field wastes fewer bytes on everyone's hard disk. @@ -1358,7 +1358,7 @@ BINARY_FIELDS = _fields( "no", hover_text=textwrap.dedent( """\ - The package is a regular package. This is the default and recommended.</p> + The package is a regular package. This is the default and recommended. Note that declaring a package to be `XB-Important: no` is the same as not having the field except omitting the field wastes fewer bytes on everyone's hard-disk. @@ -1381,7 +1381,7 @@ BINARY_FIELDS = _fields( "no", hover_text=textwrap.dedent( """\ - The package is a regular package. This is the default and recommended.</p> + The package is a regular package. This is the default and recommended. Note that declaring a package to be `Protected: no` is the same as not having the field except omitting the field wastes fewer bytes on everyone's hard-disk. @@ -1450,7 +1450,7 @@ BINARY_FIELDS = _fields( hover_text=textwrap.dedent( """\ Lists the packages that *should* be installed when this package is installed in all but - *unusual installations*.</p> + *unusual installations*. **Example**: ``` @@ -2017,7 +2017,7 @@ BINARY_FIELDS = _fields( custom_field_check=_each_value_match_regex_validation(PKGNAME_REGEX), hover_text=textwrap.dedent( """\ - Special purpose field renamed to the 64-bit time transition. + Special purpose field related to the 64-bit time transition. It is used to inform packaging helpers what the original (non-transitioned) package name was when the auto-detection is inadequate. The non-transitioned package name is then @@ -2644,7 +2644,7 @@ _DTESTSCTRL_FIELDS = _fields( hover_text=textwrap.dedent( """\ If your test only contains a shell command or two, or you want to - re-use an existing upstream test executable and just need to wrap it + reuse an existing upstream test executable and just need to wrap it with some command like `dbus-launch` or `env`, you can use this field to specify the shell command directly. It will be run under `bash -e`. This is mutually exclusive with the `Tests:` field. diff --git a/src/debputy/lsp/lsp_debian_debputy_manifest.py b/src/debputy/lsp/lsp_debian_debputy_manifest.py index d24d441..ba30c75 100644 --- a/src/debputy/lsp/lsp_debian_debputy_manifest.py +++ b/src/debputy/lsp/lsp_debian_debputy_manifest.py @@ -25,6 +25,8 @@ from lsprotocol.types import ( CompletionParams, CompletionList, CompletionItem, + DiagnosticRelatedInformation, + Location, ) from debputy.lsp.quickfixes import propose_correct_text_quick_fix from debputy.manifest_parser.base_types import DebputyDispatchableType @@ -123,7 +125,7 @@ def _word_range_at_position( @lint_diagnostics(_LANGUAGE_IDS) def _lint_debian_debputy_manifest( - _doc_reference: str, + doc_reference: str, path: str, lines: List[str], position_codec: LintCapablePositionCodec, @@ -172,94 +174,299 @@ def _lint_debian_debputy_manifest( ) else: feature_set = lsp_get_plugin_features() - root_parser = feature_set.manifest_parser_generator.dispatchable_object_parsers[ - OPARSER_MANIFEST_ROOT - ] - diagnostics.extend(_lint_content(root_parser, content, lines, position_codec)) + pg = feature_set.manifest_parser_generator + root_parser = pg.dispatchable_object_parsers[OPARSER_MANIFEST_ROOT] + diagnostics.extend( + _lint_content( + doc_reference, + pg, + root_parser, + content, + lines, + position_codec, + ) + ) return diagnostics +def _unknown_key( + key: str, + expected_keys: Iterable[str], + line: int, + col: int, + lines: List[str], + position_codec: LintCapablePositionCodec, +) -> Tuple["Diagnostic", Optional[str]]: + key_range = position_codec.range_to_client_units( + lines, + Range( + Position( + line, + col, + ), + Position( + line, + col + len(key), + ), + ), + ) + + candidates = detect_possible_typo(key, expected_keys) + extra = "" + corrected_key = None + if candidates: + extra = f' It looks like a typo of "{candidates[0]}".' + # TODO: We should be able to tell that `install-doc` and `install-docs` are the same. + # That would enable this to work in more cases. + corrected_key = candidates[0] if len(candidates) == 1 else None + + diagnostic = Diagnostic( + key_range, + f'Unknown or unsupported key "{key}".{extra}', + DiagnosticSeverity.Error, + source="debputy", + data=[propose_correct_text_quick_fix(n) for n in candidates], + ) + return diagnostic, corrected_key + + +def _conflicting_key( + uri: str, + key_a: str, + key_b: str, + key_a_line: int, + key_a_col: int, + key_b_line: int, + key_b_col: int, + lines: List[str], + position_codec: LintCapablePositionCodec, +) -> Iterable["Diagnostic"]: + key_a_range = position_codec.range_to_client_units( + lines, + Range( + Position( + key_a_line, + key_a_col, + ), + Position( + key_a_line, + key_a_col + len(key_a), + ), + ), + ) + key_b_range = position_codec.range_to_client_units( + lines, + Range( + Position( + key_b_line, + key_b_col, + ), + Position( + key_b_line, + key_b_col + len(key_b), + ), + ), + ) + yield Diagnostic( + key_a_range, + f'The "{key_a}" cannot be used with "{key_b}".', + DiagnosticSeverity.Error, + source="debputy", + related_information=[ + DiagnosticRelatedInformation( + location=Location( + uri, + key_b_range, + ), + message=f'The attribute "{key_b}" is used here.', + ) + ], + ) + + yield Diagnostic( + key_b_range, + f'The "{key_b}" cannot be used with "{key_a}".', + DiagnosticSeverity.Error, + source="debputy", + related_information=[ + DiagnosticRelatedInformation( + location=Location( + uri, + key_a_range, + ), + message=f'The attribute "{key_a}" is used here.', + ) + ], + ) + + +def _lint_attr_value( + uri: str, + attr: AttributeDescription, + pg: ParserGenerator, + value: Any, + lines: List[str], + position_codec: LintCapablePositionCodec, +) -> Iterable["Diagnostic"]: + attr_type = attr.attribute_type + orig = get_origin(attr_type) + valid_values: Sequence[Any] = tuple() + if orig == Literal: + valid_values = get_args(attr.attribute_type) + elif orig == bool or attr.attribute_type == bool: + valid_values = ("true", "false") + elif isinstance(attr_type, type) and issubclass(attr_type, DebputyDispatchableType): + parser = pg.dispatch_parser_table_for(attr_type) + yield from _lint_content( + uri, + pg, + parser, + value, + lines, + position_codec, + ) + return + + if value in valid_values: + return + # TODO: Emit diagnostic for broken values + return + + +def _lint_declarative_mapping_input_parser( + uri: str, + pg: ParserGenerator, + parser: DeclarativeMappingInputParser, + content: Any, + lines: List[str], + position_codec: LintCapablePositionCodec, +) -> Iterable["Diagnostic"]: + if not isinstance(content, CommentedMap): + return + lc = content.lc + for key, value in content.items(): + attr = parser.manifest_attributes.get(key) + line, col = lc.key(key) + if attr is None: + diag, corrected_key = _unknown_key( + key, + parser.manifest_attributes, + line, + col, + lines, + position_codec, + ) + yield diag + if corrected_key: + key = corrected_key + attr = parser.manifest_attributes.get(corrected_key) + if attr is None: + continue + + yield from _lint_attr_value( + uri, + attr, + pg, + value, + lines, + position_codec, + ) + + for forbidden_key in attr.conflicting_attributes: + if forbidden_key in content: + con_line, con_col = lc.key(forbidden_key) + yield from _conflicting_key( + uri, + key, + forbidden_key, + line, + col, + con_line, + con_col, + lines, + position_codec, + ) + for mx in parser.mutually_exclusive_attributes: + matches = content.keys() & mx + if len(matches) < 2: + continue + key, *others = list(matches) + line, col = lc.key(key) + for other in others: + con_line, con_col = lc.key(other) + yield from _conflicting_key( + uri, + key, + other, + line, + col, + con_line, + con_col, + lines, + position_codec, + ) + + def _lint_content( + uri: str, + pg: ParserGenerator, parser: DeclarativeInputParser[Any], content: Any, lines: List[str], position_codec: LintCapablePositionCodec, -) -> Iterable[Diagnostic]: +) -> Iterable["Diagnostic"]: if isinstance(parser, DispatchingParserBase): if not isinstance(content, CommentedMap): return lc = content.lc for key, value in content.items(): - if not parser.is_known_keyword(key): + is_known = parser.is_known_keyword(key) + if not is_known: line, col = lc.key(key) - key_range = position_codec.range_to_client_units( + diag, corrected_key = _unknown_key( + key, + parser.registered_keywords(), + line, + col, lines, - Range( - Position( - line, - col, - ), - Position( - line, - col + len(key), - ), - ), + position_codec, ) + yield diag + if corrected_key is not None: + key = corrected_key + is_known = True - candidates = detect_possible_typo(key, parser.registered_keywords()) - - yield Diagnostic( - key_range, - f"Unknown or unsupported key {key}", - DiagnosticSeverity.Error, - source="debputy", - data=[propose_correct_text_quick_fix(n) for n in candidates], - ) - else: + if is_known: subparser = parser.parser_for(key) assert subparser is not None - yield from _lint_content(subparser.parser, value, lines, position_codec) + yield from _lint_content( + uri, + pg, + subparser.parser, + value, + lines, + position_codec, + ) elif isinstance(parser, ListWrappedDeclarativeInputParser): if not isinstance(content, CommentedSeq): return subparser = parser.delegate for value in content: - yield from _lint_content(subparser, value, lines, position_codec) + yield from _lint_content(uri, pg, subparser, value, lines, position_codec) elif isinstance(parser, InPackageContextParser): if not isinstance(content, CommentedMap): return for v in content.values(): - yield from _lint_content(parser.delegate, v, lines, position_codec) + yield from _lint_content(uri, pg, parser.delegate, v, lines, position_codec) elif isinstance(parser, DeclarativeMappingInputParser): - if not isinstance(content, CommentedMap): - return - lc = content.lc - for key, value in content.items(): - attr = parser.manifest_attributes.get(key) - if attr is None: - line, col = lc.key(key) - key_range = position_codec.range_to_client_units( - lines, - Range( - Position( - line, - col, - ), - Position( - line, - col + len(key), - ), - ), - ) - - candidates = detect_possible_typo(key, parser.manifest_attributes) - yield Diagnostic( - key_range, - f"Unknown or unsupported key {key}", - DiagnosticSeverity.Error, - source="debputy", - data=[propose_correct_text_quick_fix(n) for n in candidates], - ) + yield from _lint_declarative_mapping_input_parser( + uri, + pg, + parser, + content, + lines, + position_codec, + ) def is_at(position: Position, lc_pos: Tuple[int, int]) -> bool: @@ -469,7 +676,7 @@ def _guess_rule_name(segments: List[Union[str, int]], idx: int) -> str: return "<Bug: unknown rule name>" -def _ecsape(v: str) -> str: +def _escape(v: str) -> str: return '"' + v.replace("\n", "\\n") + '"' @@ -479,7 +686,7 @@ def _insert_snippet(lines: List[str], server_position: Position) -> bool: line = lines[line_no] pos_rhs = line[server_position.character :] if pos_rhs and not pos_rhs.isspace(): - _info(f"No insertion: {_ecsape(line[server_position.character:])}") + _info(f"No insertion: {_escape(line[server_position.character:])}") return False lhs_ws = line[: server_position.character] lhs = lhs_ws.strip() @@ -492,17 +699,17 @@ def _insert_snippet(lines: List[str], server_position: Position) -> bool: snippet = _COMPLETION_HINT_KEY if ":" not in lhs else _COMPLETION_HINT_VALUE new_line = line[: server_position.character] + snippet elif not lhs or (lhs_ws and not lhs_ws[0].isspace()): - _info(f"Insertion of key or value: {_ecsape(line[server_position.character:])}") + _info(f"Insertion of key or value: {_escape(line[server_position.character:])}") # Respect the provided indentation snippet = _COMPLETION_HINT_KEY if ":" not in lhs else _COMPLETION_HINT_VALUE new_line = line[: server_position.character] + snippet elif lhs.isalpha() and ":" not in lhs: - _info(f"Expanding value to a key: {_ecsape(line[server_position.character:])}") + _info(f"Expanding value to a key: {_escape(line[server_position.character:])}") # Respect the provided indentation new_line = line[: server_position.character] + _COMPLETION_HINT_KEY else: c = line[server_position.character] - _info(f"Not touching line: {_ecsape(line)} -- {_ecsape(c)}") + _info(f"Not touching line: {_escape(line)} -- {_escape(c)}") return False _info(f'Evaluating complete on synthetic line: "{new_line}"') lines[line_no] = new_line diff --git a/src/debputy/lsp/lsp_debian_rules.py b/src/debputy/lsp/lsp_debian_rules.py index 86b114c..f05099d 100644 --- a/src/debputy/lsp/lsp_debian_rules.py +++ b/src/debputy/lsp/lsp_debian_rules.py @@ -1,3 +1,4 @@ +import functools import itertools import json import os @@ -152,10 +153,17 @@ def _lint_debian_rules( ) +@functools.lru_cache +def _is_project_trusted(source_root: str) -> bool: + return os.environ.get("DEBPUTY_TRUST_PROJECT", "0") == "1" + + def _run_make_dryrun( source_root: str, lines: List[str], ) -> Optional[Diagnostic]: + if not _is_project_trusted(source_root): + return None try: make_res = subprocess.run( ["make", "--dry-run", "-f", "-", "debhelper-fail-me"], diff --git a/src/debputy/lsp/vendoring/_deb822_repro/__init__.py b/src/debputy/lsp/vendoring/_deb822_repro/__init__.py index cc2b1de..0736189 100644 --- a/src/debputy/lsp/vendoring/_deb822_repro/__init__.py +++ b/src/debputy/lsp/vendoring/_deb822_repro/__init__.py @@ -150,7 +150,7 @@ Deb822ParagraphElement.as_interpreted_dict_view method. Stability of this API --------------------- -The API is subject to change based on feedback from early adoptors and beta +The API is subject to change based on feedback from early adopters and beta testers. That said, the code for valid files is unlikely to change in a backwards incompatible way. diff --git a/src/debputy/lsp/vendoring/_deb822_repro/parsing.py b/src/debputy/lsp/vendoring/_deb822_repro/parsing.py index 1a2da25..e2c638a 100644 --- a/src/debputy/lsp/vendoring/_deb822_repro/parsing.py +++ b/src/debputy/lsp/vendoring/_deb822_repro/parsing.py @@ -3008,7 +3008,7 @@ class Deb822FileElement(Deb822Element): """Inserts a paragraph into the file at the given "index" of paragraphs Note that if the index is between two paragraphs containing a "free - floating" comment (e.g. paragrah/start-of-file, empty line, comment, + floating" comment (e.g. paragraph/start-of-file, empty line, comment, empty line, paragraph) then it is unspecified which "side" of the comment the new paragraph will appear and this may change between versions of python-debian. diff --git a/src/debputy/lsp/vendoring/_deb822_repro/tokens.py b/src/debputy/lsp/vendoring/_deb822_repro/tokens.py index 5db991a..6697a2c 100644 --- a/src/debputy/lsp/vendoring/_deb822_repro/tokens.py +++ b/src/debputy/lsp/vendoring/_deb822_repro/tokens.py @@ -171,7 +171,7 @@ class Deb822Token(Locatable): return self._text def size(self, *, skip_leading_comments: bool = False) -> Range: - # As tokens are an atomtic unit + # As tokens are an atomic unit token_size = self._token_size if token_size is not None: return token_size diff --git a/src/debputy/lsp/vendoring/_deb822_repro/types.py b/src/debputy/lsp/vendoring/_deb822_repro/types.py index 7b78024..181f5c9 100644 --- a/src/debputy/lsp/vendoring/_deb822_repro/types.py +++ b/src/debputy/lsp/vendoring/_deb822_repro/types.py @@ -61,7 +61,7 @@ try: """ FormatterCallback.__doc__ = """\ Formatter callback used with the round-trip safe parser - + See debian._repro_deb822.formatter.format_field for details """ except AttributeError: diff --git a/src/debputy/manifest_parser/declarative_parser.py b/src/debputy/manifest_parser/declarative_parser.py index f18dc1c..850cfa8 100644 --- a/src/debputy/manifest_parser/declarative_parser.py +++ b/src/debputy/manifest_parser/declarative_parser.py @@ -652,7 +652,7 @@ class DebputyParseHint: Generally, this type hint must be placed on the **source** format. Any source attribute matching the parsed format will be ignored. - Mind the assymmetry: The annotation is placed in the **source** format while `debputy` looks at + Mind the asymmetry: The annotation is placed in the **source** format while `debputy` looks at the type of the target attribute to determine if it counts as path. """ return NOT_PATH_HINT @@ -795,7 +795,7 @@ class ParserGenerator: into: ["my-pkg"] ``` - While this is sufficient for programmers, it is a bit ridig for the packager writing the manifest. Therefore, + While this is sufficient for programmers, it is a bit rigid for the packager writing the manifest. Therefore, you can also provide a TypedDict describing the input, enabling more flexibility: >>> class InstallDocsRule(DebputyParsedContent): diff --git a/src/debputy/plugin/api/spec.py b/src/debputy/plugin/api/spec.py index 5d7a261..dba4523 100644 --- a/src/debputy/plugin/api/spec.py +++ b/src/debputy/plugin/api/spec.py @@ -1288,7 +1288,7 @@ class VirtualPath: :param use_fs_path_mode: If True, any changes to the mode on the physical FS path will be recorded as the desired mode of the file when the contextmanager ends. The provided FS path with start with the current mode when `use_fs_path_mode` is True. Otherwise, `debputy` will - ignore the mode of the file system entry and re-use its own current mode + ignore the mode of the file system entry and reuse its own current mode definition. :return: A Context manager that upon entering provides the path to a muable (copy) of this path's `fs_path` attribute. The file on the underlying path may be mutated however diff --git a/src/debputy/plugin/debputy/binary_package_rules.py b/src/debputy/plugin/debputy/binary_package_rules.py index 14d9b91..29c6136 100644 --- a/src/debputy/plugin/debputy/binary_package_rules.py +++ b/src/debputy/plugin/debputy/binary_package_rules.py @@ -96,7 +96,7 @@ def register_binary_package_rules(api: DebputyPluginInitializerProvider) -> None `dpkg-gencontrol`. The value for the `binary-version` key is a string that defines the binary version. Generally, - you will want it to contain one of the versioned related substitution variables such as + you will want it to contain one of the versioned related substitution variables such as `{{DEB_VERSION_UPSTREAM_REVISION}}`. Otherwise, you will have to remember to bump the version manually with each upload as versions cannot be reused and the package would not support binNMUs either. @@ -257,7 +257,7 @@ def register_binary_package_rules(api: DebputyPluginInitializerProvider) -> None manager integration determines that `reload` is not supported. - `restart`: During an upgrade, `restart` the service post upgrade. The service will be left running during the upgrade process. - - `stop-then-start`: Stop the service before the upgrade, preform the upgrade and + - `stop-then-start`: Stop the service before the upgrade, perform the upgrade and then start the service. """ ), diff --git a/src/debputy/plugin/debputy/private_api.py b/src/debputy/plugin/debputy/private_api.py index 3b5087b..6d60333 100644 --- a/src/debputy/plugin/debputy/private_api.py +++ b/src/debputy/plugin/debputy/private_api.py @@ -897,7 +897,7 @@ def register_install_rules(api: DebputyPluginInitializerProvider) -> None: With these two things in mind, it behaves just like the `install` rule. Note: It is often worth considering to use a more specialized version of the `install-docs` - rule when one such is available. If you are looking to install an example or a manpage, + rule when one such is available. If you are looking to install an example or a man page, consider whether `install-examples` or `install-man` might be a better fit for your use-case. """ @@ -1066,18 +1066,18 @@ def register_install_rules(api: DebputyPluginInitializerProvider) -> None: _install_man_rule_handler, source_format=_with_alt_form(ParsedInstallManpageRuleSourceFormat), inline_reference_documentation=reference_documentation( - title="Install manpages (`install-man`)", + title="Install man pages (`install-man`)", description=textwrap.dedent( """\ - Install rule for installing manpages similar to `dh_installman`. It is a shorthand + Install rule for installing man pages similar to `dh_installman`. It is a shorthand over the generic `install` rule with the following key features: 1) The rule can only match files (notably, symlinks cannot be matched by this rule). - 2) The `dest-dir` is computed per source file based on the manpage's section and + 2) The `dest-dir` is computed per source file based on the man page's section and language. 3) The `into` parameter can be omitted as long as there is a exactly one non-`udeb` package listed in `debian/control`. - 4) The rule comes with manpage specific attributes such as `language` and `section` + 4) The rule comes with man page specific attributes such as `language` and `section` for when the auto-detection is insufficient. 5) The rule comes with pre-defined conditional logic for skipping the rule under `DEB_BUILD_OPTIONS=nodoc`, so you do not have to write that conditional yourself. @@ -1111,7 +1111,7 @@ def register_install_rules(api: DebputyPluginInitializerProvider) -> None: textwrap.dedent( """\ Either a package name or a list of package names for which these paths should be - installed as manpages. This key is conditional on whether there are multiple (non-`udeb`) + installed as man pages. This key is conditional on whether there are multiple (non-`udeb`) binary packages listed in `debian/control`. When there is only one (non-`udeb`) binary package, then that binary is the default for `into`. Otherwise, the key is required. """ @@ -1122,7 +1122,7 @@ def register_install_rules(api: DebputyPluginInitializerProvider) -> None: textwrap.dedent( """\ If provided, it must be an integer between 1 and 9 (both inclusive), defining the - section the manpages belong overriding any auto-detection that `debputy` would + section the man pages belong overriding any auto-detection that `debputy` would have performed. """ ), @@ -1134,8 +1134,8 @@ def register_install_rules(api: DebputyPluginInitializerProvider) -> None: If provided, it must be either a 2 letter language code (such as `de`), a 5 letter language + dialect code (such as `pt_BR`), or one of the special keywords `C`, `derive-from-path`, or `derive-from-basename`. The default is `derive-from-path`. - - When `language` is `C`, then the manpages are assumed to be "untranslated". - - When `language` is a language code (with or without dialect), then all manpages + - When `language` is `C`, then the man pages are assumed to be "untranslated". + - When `language` is a language code (with or without dialect), then all man pages matched will be assumed to be translated to that concrete language / dialect. - When `language` is `derive-from-path`, then `debputy` attempts to derive the language from the path (`man/<language>/man<section>`). This matches the @@ -1216,7 +1216,7 @@ def register_install_rules(api: DebputyPluginInitializerProvider) -> None: requested by the packager or a search directory that `debputy` provided automatically (such as `debian/tmp`). Listing other paths will make `debputy` report an error. - - Note that the `path` or `paths` must match at least one entry in + - Note that the `path` or `paths` must match at least one entry in any of the search directories unless *none* of the search directories exist (or the condition in `required-when` evaluates to false). When none of the search directories exist, the discard rule is silently @@ -2604,7 +2604,7 @@ def _install_man_rule_handler( ) if section is None and any(s.raw_match_rule.endswith(".gz") for s in sources): raise ManifestParseException( - "Sorry, compressed manpages are not supported without an explicit `section` definition at the moment." + "Sorry, compressed man pages are not supported without an explicit `section` definition at the moment." " This limitation may be removed in the future. Problematic definition from" f' {attribute_path["sources"]}' ) diff --git a/tests/lint_tests/__init__.py b/tests/lint_tests/__init__.py new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/tests/lint_tests/__init__.py diff --git a/tests/lint_tests/conftest.py b/tests/lint_tests/conftest.py new file mode 100644 index 0000000..d08f5ca --- /dev/null +++ b/tests/lint_tests/conftest.py @@ -0,0 +1,30 @@ +import pytest + +from debputy.lsp.lsp_features import lsp_set_plugin_features +from debputy.plugin.api.feature_set import PluginProvidedFeatureSet +from debputy.util import setup_logging + +try: + from lsprotocol.types import Diagnostic + + HAS_LSPROTOCOL = True +except ImportError: + HAS_LSPROTOCOL = False + + +@pytest.fixture(scope="session", autouse=True) +def enable_logging() -> None: + if not HAS_LSPROTOCOL: + pytest.skip("Missing python3-lsprotocol") + setup_logging(reconfigure_logging=True) + + +@pytest.fixture(autouse=True) +def setup_feature_set( + debputy_plugin_feature_set: PluginProvidedFeatureSet, +) -> None: + lsp_set_plugin_features(debputy_plugin_feature_set) + try: + yield + finally: + lsp_set_plugin_features(None) diff --git a/tests/lint_tests/lint_tutil.py b/tests/lint_tests/lint_tutil.py new file mode 100644 index 0000000..d4f654c --- /dev/null +++ b/tests/lint_tests/lint_tutil.py @@ -0,0 +1,71 @@ +import collections +from typing import List, Optional, Mapping, Any + +import pytest + +from debputy.linting.lint_util import LinterImpl, LinterPositionCodec + +try: + from lsprotocol.types import Diagnostic, DiagnosticSeverity +except ImportError: + pass + + +try: + from Levenshtein import distance + + HAS_LEVENSHTEIN = True +except ImportError: + HAS_LEVENSHTEIN = False + + +LINTER_POSITION_CODEC = LinterPositionCodec() + + +def requires_levenshtein(func: Any) -> Any: + return pytest.mark.skipif( + not HAS_LEVENSHTEIN, reason="Missing python3-levenshtein" + )(func) + + +def _check_diagnostics( + diagnostics: Optional[List["Diagnostic"]], +) -> Optional[List["Diagnostic"]]: + if diagnostics: + for diagnostic in diagnostics: + assert diagnostic.severity is not None + return diagnostics + + +def run_linter( + path: str, lines: List[str], linter: LinterImpl +) -> Optional[List["Diagnostic"]]: + uri = f"file://{path}" + return _check_diagnostics(linter(uri, path, lines, LINTER_POSITION_CODEC)) + + +def exactly_one_diagnostic(diagnostics: Optional[List["Diagnostic"]]) -> "Diagnostic": + assert diagnostics and len(diagnostics) == 1 + return diagnostics[0] + + +def by_range_sort_key(diagnostic: Diagnostic) -> Any: + start_pos = diagnostic.range.start + end_pos = diagnostic.range.end + return start_pos.line, start_pos.character, end_pos.line, end_pos.character + + +def group_diagnostics_by_severity( + diagnostics: Optional[List["Diagnostic"]], +) -> Mapping["DiagnosticSeverity", List["Diagnostic"]]: + if not diagnostics: + return {} + + by_severity = collections.defaultdict(list) + + for diagnostic in sorted(diagnostics, key=by_range_sort_key): + severity = diagnostic.severity + assert severity is not None + by_severity[severity].append(diagnostic) + + return by_severity diff --git a/tests/lint_tests/test_lint_dctrl.py b/tests/lint_tests/test_lint_dctrl.py new file mode 100644 index 0000000..cc2758e --- /dev/null +++ b/tests/lint_tests/test_lint_dctrl.py @@ -0,0 +1,117 @@ +import textwrap +from typing import List, Optional, Callable + +import pytest + +from debputy.lsp.lsp_debian_control import _lint_debian_control +from lint_tests.lint_tutil import ( + run_linter, + group_diagnostics_by_severity, + requires_levenshtein, + exactly_one_diagnostic, +) + +try: + from lsprotocol.types import Diagnostic, DiagnosticSeverity +except ImportError: + pass + + +TestLinter = Callable[[List[str]], Optional[List["Diagnostic"]]] + + +@pytest.fixture +def line_linter() -> TestLinter: + path = "/nowhere/debian/control" + + def _linter(lines: List[str]) -> Optional[List["Diagnostic"]]: + return run_linter(path, lines, _lint_debian_control) + + return _linter + + +def test_dctrl_lint(line_linter: TestLinter) -> None: + lines = textwrap.dedent( + """\ + Source: foo + Some-Other-Field: bar + Build-Depends: debhelper-compat (= 13) + + Package: foo + Architecture: all + # Unknown section + Section: base + """ + ).splitlines(keepends=True) + + diagnostics = line_linter(lines) + by_severity = group_diagnostics_by_severity(diagnostics) + # This example triggers errors and warnings, but no hint of info + assert DiagnosticSeverity.Error in by_severity + assert DiagnosticSeverity.Warning in by_severity + + assert DiagnosticSeverity.Hint not in by_severity + assert DiagnosticSeverity.Information not in by_severity + + errors = by_severity[DiagnosticSeverity.Error] + print(errors) + assert len(errors) == 3 + + first_error, second_error, third_error = errors + + msg = "Stanza is missing field Standards-Version" + assert first_error.message == msg + assert f"{first_error.range}" == "0:0-1:0" + + msg = "Stanza is missing field Maintainer" + assert second_error.message == msg + assert f"{second_error.range}" == "0:0-1:0" + + msg = "Stanza is missing field Priority" + assert third_error.message == msg + assert f"{third_error.range}" == "4:0-5:0" + + warnings = by_severity[DiagnosticSeverity.Warning] + assert len(warnings) == 2 + + first_warn, second_warn = warnings + + msg = "Stanza is missing field Description" + assert first_warn.message == msg + assert f"{first_warn.range}" == "4:0-5:0" + + msg = 'The value "base" is not supported in Section.' + assert second_warn.message == msg + assert f"{second_warn.range}" == "8:9-8:13" + + +@requires_levenshtein +def test_dctrl_lint_typos(line_linter: TestLinter) -> None: + lines = textwrap.dedent( + """\ + Source: foo + Standards-Version: 4.5.2 + Priority: optional + Section: devel + Maintainer: Jane Developer <jane@example.com> + # Typo + Build-Dpends: debhelper-compat (= 13) + + Package: foo + Architecture: all + 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) + diag = exactly_one_diagnostic(diagnostics) + + msg = 'The "Build-Dpends" looks like a typo of "Build-Depends".' + assert diag.message == msg + assert diag.severity == DiagnosticSeverity.Warning + assert f"{diag.range}" == "6:0-6:12" diff --git a/tests/lint_tests/test_lint_debputy.py b/tests/lint_tests/test_lint_debputy.py new file mode 100644 index 0000000..74977d0 --- /dev/null +++ b/tests/lint_tests/test_lint_debputy.py @@ -0,0 +1,181 @@ +import textwrap +from typing import List, Optional, Callable + +import pytest + +from debputy.lsp.lsp_debian_debputy_manifest import _lint_debian_debputy_manifest +from lint_tests.lint_tutil import ( + run_linter, + group_diagnostics_by_severity, + requires_levenshtein, +) + +try: + from lsprotocol.types import Diagnostic, DiagnosticSeverity +except ImportError: + pass + + +TestLinter = Callable[[List[str]], Optional[List["Diagnostic"]]] + + +@pytest.fixture +def line_linter() -> TestLinter: + path = "/nowhere/debian/debputy.manifest" + + def _linter(lines: List[str]) -> Optional[List["Diagnostic"]]: + return run_linter(path, lines, _lint_debian_debputy_manifest) + + return _linter + + +def test_debputy_lint_unknown_keys(line_linter: TestLinter) -> None: + lines = textwrap.dedent( + """\ + manifest-version: 0.1 + installations: + - install-something: + sources: + - abc + - def + - install-docs: + source: foo + puff: true # Unknown keyword (assuming install-docs) + when: + negated: cross-compiling + - install-docs: + source: bar + when: ross-compiling # Typo of "cross-compiling"; FIXME not caught + packages: + foo: + blah: qwe # Unknown keyword + """ + ).splitlines(keepends=True) + + diagnostics = line_linter(lines) + by_severity = group_diagnostics_by_severity(diagnostics) + # This example triggers errors only + assert DiagnosticSeverity.Error in by_severity + + assert DiagnosticSeverity.Warning not in by_severity + assert DiagnosticSeverity.Hint not in by_severity + assert DiagnosticSeverity.Information not in by_severity + + errors = by_severity[DiagnosticSeverity.Error] + print(errors) + assert len(errors) == 4 + + first_error, second_error, third_error, fourth_error = errors + + msg = 'Unknown or unsupported key "install-something".' + assert first_error.message == msg + assert f"{first_error.range}" == "2:2-2:19" + + msg = 'Unknown or unsupported key "puff".' + assert second_error.message == msg + assert f"{second_error.range}" == "8:4-8:8" + + msg = 'Unknown or unsupported key "negated".' + assert third_error.message == msg + assert f"{third_error.range}" == "10:6-10:13" + + msg = 'Unknown or unsupported key "blah".' + assert fourth_error.message == msg + assert f"{fourth_error.range}" == "16:4-16:8" + + +@requires_levenshtein +def test_debputy_lint_unknown_keys_spelling(line_linter: TestLinter) -> None: + lines = textwrap.dedent( + """\ + manifest-version: 0.1 + installations: + - install-dcoss: # typo + sources: + - abc + - def + puff: true # Unknown keyword (assuming install-docs) + when: + nut: cross-compiling # Typo of "not" + - install-docs: + source: bar + when: ross-compiling # Typo of "cross-compiling"; FIXME not caught + """ + ).splitlines(keepends=True) + + diagnostics = line_linter(lines) + by_severity = group_diagnostics_by_severity(diagnostics) + # This example triggers errors only + assert DiagnosticSeverity.Error in by_severity + + assert DiagnosticSeverity.Warning not in by_severity + assert DiagnosticSeverity.Hint not in by_severity + assert DiagnosticSeverity.Information not in by_severity + + errors = by_severity[DiagnosticSeverity.Error] + print(errors) + assert len(errors) == 3 + + first_error, second_error, third_error = errors + + msg = 'Unknown or unsupported key "install-dcoss". It looks like a typo of "install-docs".' + assert first_error.message == msg + assert f"{first_error.range}" == "2:2-2:15" + + msg = 'Unknown or unsupported key "puff".' + assert second_error.message == msg + assert f"{second_error.range}" == "6:4-6:8" + + msg = 'Unknown or unsupported key "nut". It looks like a typo of "not".' + assert third_error.message == msg + assert f"{third_error.range}" == "8:6-8:9" + + +def test_debputy_lint_conflicting_keys(line_linter: TestLinter) -> None: + lines = textwrap.dedent( + """\ + manifest-version: 0.1 + installations: + - install-docs: + sources: + - foo + - bar + as: baz # Conflicts with "sources" (#85) + - install: + source: foo + sources: # Conflicts with "source" (#85) + - bar + - baz + """ + ).splitlines(keepends=True) + + diagnostics = line_linter(lines) + by_severity = group_diagnostics_by_severity(diagnostics) + # This example triggers errors only + assert DiagnosticSeverity.Error in by_severity + + assert DiagnosticSeverity.Warning not in by_severity + assert DiagnosticSeverity.Hint not in by_severity + assert DiagnosticSeverity.Information not in by_severity + + errors = by_severity[DiagnosticSeverity.Error] + print(errors) + assert len(errors) == 4 + + first_error, second_error, third_error, fourth_error = errors + + msg = 'The "sources" cannot be used with "as".' + assert first_error.message == msg + assert f"{first_error.range}" == "3:4-3:11" + + msg = 'The "as" cannot be used with "sources".' + assert second_error.message == msg + assert f"{second_error.range}" == "6:4-6:6" + + msg = 'The "source" cannot be used with "sources".' + assert third_error.message == msg + assert f"{third_error.range}" == "8:4-8:10" + + msg = 'The "sources" cannot be used with "source".' + assert fourth_error.message == msg + assert f"{fourth_error.range}" == "9:4-9:11" diff --git a/tests/lsp_tests/conftest.py b/tests/lsp_tests/conftest.py index 2aff4a2..ec12e9a 100644 --- a/tests/lsp_tests/conftest.py +++ b/tests/lsp_tests/conftest.py @@ -24,14 +24,13 @@ except ImportError: HAS_PYGLS = False -@pytest.fixture(scope="session") +@pytest.fixture(scope="session", autouse=True) def enable_logging() -> None: setup_logging(log_only_to_stderr=True, reconfigure_logging=True) @pytest.fixture() def ls( - enable_logging, debputy_plugin_feature_set: PluginProvidedFeatureSet, ) -> "LanguageServer": if not HAS_PYGLS: diff --git a/tests/test_apply_compression.py b/tests/test_apply_compression.py index 70817f9..e1e621b 100644 --- a/tests/test_apply_compression.py +++ b/tests/test_apply_compression.py @@ -9,7 +9,7 @@ def test_apply_compression(): [ virtual_path_def( "./usr/share/man/man1/foo.1", - materialized_content="manpage content", + materialized_content="man page content", ), virtual_path_def("./usr/share/man/man1/bar.1", link_target="foo.1"), virtual_path_def( diff --git a/tests/test_migrations.py b/tests/test_migrations.py index dc07d4c..cbf3f79 100644 --- a/tests/test_migrations.py +++ b/tests/test_migrations.py @@ -1099,7 +1099,7 @@ def test_migrate_installman_file( """ ) expected_warnings = [ - 'Detected manpages that might rely on "derive-from-basename" logic. Please double check' + 'Detected man pages that might rely on "derive-from-basename" logic. Please double check' " that the generated `install-man` rules are correct" ] _verify_migrator_generates_parsable_manifest( |