From 4801d340318c848d79aec638333312240357ff17 Mon Sep 17 00:00:00 2001 From: Abel Soares Siqueira Date: Fri, 25 Oct 2024 17:45:39 +0200 Subject: [PATCH] Add ExplicitImports checks to pre-commit in the template Create questions CheckExplicitImports and ExplicitImportsChecklist. Closes #349, #217 --- .pre-commit-config.yaml | 2 +- CHANGELOG.md | 1 + copier/code-quality.yml | 38 +++++++++++++++++++ docs/src/10-full-guide.md | 11 ++++++ src/debug/Data.jl | 2 + src/debug/helper.jl | 2 + template/.github/workflows/Lint.yml.jinja | 6 ++- ...%}.pre-commit-config.yaml{% endif %}.jinja | 8 ++++ 8 files changed, 68 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 9ba264c..4b9a7ac 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -45,7 +45,7 @@ repos: hooks: - id: yamllint - repo: https://github.com/ericphanson/ExplicitImports.jl - rev: v1.10.0 + rev: v1.10.1 hooks: - id: explicit-imports name: ExplicitImports checks diff --git a/CHANGELOG.md b/CHANGELOG.md index 7af9ded..8d48f78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ BREAKING NOTICE: ### Added - Create a new strategy to allow users to install all recommended and get asked additional questions. +- New questions: `CheckExplicitImports` and `ExplicitImportsChecklist`, to determine whether to check for using vs import and public API usage, and which checks to perform (#349) ### Changed diff --git a/copier/code-quality.yml b/copier/code-quality.yml index e0b06f0..018ddc0 100644 --- a/copier/code-quality.yml +++ b/copier/code-quality.yml @@ -72,3 +72,41 @@ ConfigIndentation: This does **NOT** enforce indentation by itself, you still need tools to indent these. `pre-commit` is the recommended way to run these tools. For existing packages, this will be inferred from the indent value in the `.JuliaFormatter.toml` file. + +CheckExplicitImports: + when: "{{ AnswerStrategy == 'ask' or AnswerStrategy == 'recommended-ask' }}" + type: bool + help: Add ExplicitImports check to pre-commit (Check correct usage of using vs import and public API usage. See https://github.com/ericphanson/ExplicitImports.jl) + default: false + description: | + ExplicitImports performs a few checks to determine if this package is using other packages correctly. + See https://github.com/ericphanson/ExplicitImports.jl + Defaults to false, since many Julia packages have not adopted the idea yet. + +ExplicitImportsChecklist: + when: "{{ CheckExplicitImports }}" + type: str + help: Checklist for ExplicitImports (Comma-separated list to be passed to ExplicitImports' --checklist. Defaults to a reasonable strictness. Use `all` to run everything or see all options in https://github.com/ericphanson/ExplicitImports.jl?tab=readme-ov-file#command-line-usage) + default: "exclude_all_qualified_accesses_are_public" + description: | + ExplicitImports performs various checks. Some depend on the imported + packages fixing the definition of public API to Julia's latest definitions, + so using "all" is prone to failures until most Julia packages adopt the new + definition. + + Valid values for each check are: + - Individual checks: + - all_explicit_imports_are_public, + - all_qualified_accesses_are_public, + - all_explicit_imports_via_owners, + - all_qualified_accesses_via_owners, + - no_implicit_imports, + - no_self_qualified_accesses, + - no_stale_explicit_imports + - Select all checks: all + - Exclude a check: prepend an individual check with 'exclude_' + + The selection logic is performed in the order given. + If you pass only exclusions, it will assume that it starts from a complete list, and then excludes. + If you pass any individual checks, it will assume that it starts from an empty list, and then includes. + Passing both individual and exclusion checks does not make sense. diff --git a/docs/src/10-full-guide.md b/docs/src/10-full-guide.md index d54e65f..dd505ae 100644 --- a/docs/src/10-full-guide.md +++ b/docs/src/10-full-guide.md @@ -68,6 +68,17 @@ pkg> activate pkg> add JuliaFormatter ``` +#### ExplicitImports + +When `CheckExplicitImports` is true, the code will be checked with [ExplicitImports.jl](https://github.com/ericphanson/ExplicitImports.jl) for correct usage of using vs imports and public API usage. +Like JuliaFormatter, ExplicitImports needs to be explicitly installed in your global environment. + +```julia-repl +julia> # Press ] +pkg> activate +pkg> add ExplicitImports +``` + ## Install BestieTemplate (or copier) To use the template, we recommend installing the package `BestieTemplate.jl` globally: diff --git a/src/debug/Data.jl b/src/debug/Data.jl index 45926e2..9f00b92 100644 --- a/src/debug/Data.jl +++ b/src/debug/Data.jl @@ -47,6 +47,8 @@ const optional_questions_with_default = Dict( "AddPrecommit" => true, "MarkdownIndentation" => 2, "ConfigIndentation" => 2, + "CheckExplicitImports" => true, + "ExplicitImportsChecklist" => "all", "AutoIncludeTests" => true, "JuliaMinCIVersion" => "lts", "AddMacToCI" => true, diff --git a/src/debug/helper.jl b/src/debug/helper.jl index 592428f..0ad69fe 100644 --- a/src/debug/helper.jl +++ b/src/debug/helper.jl @@ -83,6 +83,8 @@ function dbg_generate( ) end +dbg_generate(_data = Dict(); kwargs...) = dbg_generate(rand_pkg_name(), _data; kwargs...) + """ dbg_apply([dst_path, data]; data_choice=:minimum) diff --git a/template/.github/workflows/Lint.yml.jinja b/template/.github/workflows/Lint.yml.jinja index f1d6de1..05dd001 100644 --- a/template/.github/workflows/Lint.yml.jinja +++ b/template/.github/workflows/Lint.yml.jinja @@ -27,8 +27,12 @@ jobs: version: "1" - name: Use Julia cache uses: julia-actions/cache@v2 - - name: Install JuliaFormatter.jl + - name: Install Julia packages + {% if CheckExplicitImports -%} + run: julia -e 'using Pkg; pkg"add CheckExplicitImports, JuliaFormatter"' + {% else -%} run: julia -e 'using Pkg; pkg"add JuliaFormatter"' + {% endif -%} - name: Hack for setup-python cache # https://github.com/actions/setup-python/issues/807 run: touch requirements.txt - name: Setup Python diff --git a/template/{% if AddPrecommit %}.pre-commit-config.yaml{% endif %}.jinja b/template/{% if AddPrecommit %}.pre-commit-config.yaml{% endif %}.jinja index 4300e21..18dfde8 100644 --- a/template/{% if AddPrecommit %}.pre-commit-config.yaml{% endif %}.jinja +++ b/template/{% if AddPrecommit %}.pre-commit-config.yaml{% endif %}.jinja @@ -43,6 +43,14 @@ repos: rev: v1.35.1 hooks: - id: yamllint + {% if CheckExplicitImports -%} + - repo: https://github.com/ericphanson/ExplicitImports.jl + rev: v1.10.1 + hooks: + - id: explicit-imports + name: ExplicitImports checks + args: [--print, --checklist, {{ ExplicitImportsChecklist }}] + {% endif -%} - repo: https://github.com/domluna/JuliaFormatter.jl rev: v1.0.61 hooks: