From 24b0de37953082572566e9bc2409743cb7831b19 Mon Sep 17 00:00:00 2001 From: Abel Soares Siqueira Date: Sun, 6 Oct 2024 23:16:20 +0200 Subject: [PATCH] Expand CLI functionality in `main`, make available on earlier Julia versions, and add `.pre-commit-config.yaml` (#86) * Create script and pre-commit hook Update the CLI at src/main.jl to allow running the checks. The checks can be defined based on parsed arguments. Create scripts/explicit-imports.jl to pass ARGS to the CLI when running pre-commit. Create a pre-commit hook that runs the script. Move the documentation of CLI from docs/api.md to the README. Expands the CLI documentation with information on how to run the pre-commit hooks. Closes #85 * fix typo (`individial` -> `individual`) * rename files `main.jl` -> `cli.jl` * reorganize docs a little bit * reduce confusion by tweaking word elsewhere * don't show stacktrace in CLI * Revert "rename files `main.jl` -> `cli.jl`" This reverts commit d1ef007d83a967b103a3bcaf829948377b21f57c. * rename `cli` to `main` * only call `activate_and_load` once * all caps globals * exit manually * bump version --------- Co-authored-by: Eric Hanson <5846501+ericphanson@users.noreply.github.com> --- .pre-commit-hooks.yaml | 6 ++ Project.toml | 2 +- README.md | 100 ++++++++++++++++++++++++ docs/src/api.md | 27 +------ scripts/explicit-imports.jl | 4 + src/ExplicitImports.jl | 11 +-- src/main.jl | 150 ++++++++++++++++++++++++++++++++---- test/main.jl | 64 ++++++++++----- test/runtests.jl | 6 +- 9 files changed, 300 insertions(+), 70 deletions(-) create mode 100644 .pre-commit-hooks.yaml create mode 100755 scripts/explicit-imports.jl diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml new file mode 100644 index 0000000..44afe7a --- /dev/null +++ b/.pre-commit-hooks.yaml @@ -0,0 +1,6 @@ +- id: explicit-imports + name: ExplicitImports checks + entry: ./scripts/explicit-imports.jl --check + files: ^src.*\.jl$ + language: script + pass_filenames: false diff --git a/Project.toml b/Project.toml index f19ff48..8436f09 100644 --- a/Project.toml +++ b/Project.toml @@ -1,7 +1,7 @@ name = "ExplicitImports" uuid = "7d51a73a-1435-4ff3-83d9-f097790105c7" authors = ["Eric P. Hanson"] -version = "1.9.0" +version = "1.10.0" [deps] AbstractTrees = "1520ce14-60c1-5f80-bbc7-55ef81b5835c" diff --git a/README.md b/README.md index c311ef0..46f3c2e 100644 --- a/README.md +++ b/README.md @@ -102,6 +102,106 @@ Note the paths of course will differ depending on the location of the code on yo This can be handy for debugging; if you find that in fact ExplicitImports thinks a local variable is a global from another module, please file an issue and include the code snippet! +## Command-line usage + +ExplicitImports provides a `main` function to facilitate using ExplicitImports directly from the command line. For example, + +```bash +julia /scripts/explicit-imports.jl path_to_package +``` +or + +```bash +./scripts/explicit-imports.jl path_to_package +``` +from this directory. + +Alternatively, one can use the `main` function directly: + +```bash +julia -e 'using ExplicitImports: main;maini(["--print", "--checklist", "exclude_all_qualified_accesses_are_public"])' +``` + +On Julia v1.12+, one can use the syntax `julia -m ExplicitImports path` to run ExplicitImports on a particular path (defaulting to the current working directory). See [here](https://docs.julialang.org/en/v1.12-dev/NEWS/#Command-line-option-changes) for the `-m` flag. ExplicitImports.jl must be installed in the project you start Julia with (e.g. in your v1.12 default environment), and the target package to analyze must be installable on the same version of Julia (e.g. no out-of-date Manifest.toml present in the package environment). + +For example, using [`juliaup`](https://github.com/JuliaLang/juliaup)'s `nightly` feature, one can run ExplicitImports on v1.12 as follows. + +```bash +julia +nightly -m ExplicitImports --print --checklist exclude_all_qualified_accesses_are_public +``` + +To see all the options, use one of: + +```bash +julia +nightly -m ExplicitImports --help +julia /scripts/explicit-imports.jl --help +julia -e 'using ExplicitImports: main; exit(main(["--help"]))' +``` + +The output should be something like: + +```man +NAME + ExplicitImports.main - analyze a package's namespace + +SYNOPSIS + julia -m ExplicitImports [OPTIONS] + +DESCRIPTION + `ExplicitImports.main` (typically invoked as `julia -m ExplicitImports`) + analyzes a package's imports and qualified accesses, and prints the results. + +OPTIONS + + Path to the root directory of the package (default: pwd) + --help + Show this message + --check + Run checks instead of printing. If --checklist is not specified, all checks are run + --checklist ,... + Run checks specified by ,,... + This will imply --check. + + 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. +``` + +## Pre-commit hooks + +Another way to use ExplicitImports is with [pre-commit](https://pre-commit.com/). +Simply add the following to `.pre-commit-config.yaml`: + +```yaml +- repo: https://github.com/ericphanson/ExplicitImports.jl + rev: v1.10.0 + hooks: + - id: explicit-imports + args: [--print,--checklist,"exclude_all_qualified_accesses_are_public"] +``` + +The hook will run a selection of the tests and fail if any of them fail. + +This simply invokes the `ExplicitImports.main` with the `--check` flag (see the previous section), and additional valid arguments may be passed with the `args` parameter as shown. + +Note that the `--print` argument will print the explicit_imports, which might be useful for fixing the issues. +The issues are only shown if the checks fail, or if you run pre-commit with `--verbose`. + +The `--checklist` argument allows you to specify which checks to run. If omitted, all checks are run. + ## Limitations ### Some tricky scoping situations are not handled correctly diff --git a/docs/src/api.md b/docs/src/api.md index 8b8dc58..d2995b0 100644 --- a/docs/src/api.md +++ b/docs/src/api.md @@ -1,6 +1,6 @@ # API -The main entrypoint for interactive use is [`print_explicit_imports`](@ref). ExplicitImports.jl API also includes several other functions to provide programmatic access to the information gathered by the package, as well as utilities to use in regression testing. +The standard entrypoint for interactive use is [`print_explicit_imports`](@ref). ExplicitImports.jl API also includes several other functions to provide programmatic access to the information gathered by the package, as well as utilities to use in regression testing. ## Detecting implicit imports which could be made explicit @@ -68,28 +68,3 @@ explicit_imports_nonrecursive improper_qualified_accesses_nonrecursive improper_explicit_imports_nonrecursive ``` - -## Usage from the command line - -On Julia v1.12+, one can use the syntax `julia -m ExplicitImports` to run ExplicitImports on a particular path (defaulting to the current working directory). See [here](https://docs.julialang.org/en/v1.12-dev/NEWS/#Command-line-option-changes) for the `-m` flag. ExplicitImports.jl must be installed in the project you start Julia with (e.g. in your v1.12 default environment), and the target package to analyze must be installable on the same version of Julia (e.g. no out-of-date Manifest.toml present in the package environment). - -For example, using [`juliaup`](https://github.com/JuliaLang/juliaup)'s `nightly` feature, one can run ExplicitImports on v1.12 as follows. - -```man -❯ julia +nightly -m ExplicitImports --help -NAME - ExplicitImports.main - analyze a package's namespace - -SYNOPSIS - julia -m ExplicitImports - -DESCRIPTION - `ExplicitImports.main` (typically invoked as `julia -m ExplicitImports`) - analyzes a package's imports and qualified accesses, and prints the results. - -OPTIONS - - Path to the root directory of the package (default: pwd) - --help - Show this message -``` diff --git a/scripts/explicit-imports.jl b/scripts/explicit-imports.jl new file mode 100755 index 0000000..9b1cd2d --- /dev/null +++ b/scripts/explicit-imports.jl @@ -0,0 +1,4 @@ +#!/usr/bin/env julia +using ExplicitImports: main + +exit(main(ARGS)) diff --git a/src/ExplicitImports.jl b/src/ExplicitImports.jl index c1a739a..178e11a 100644 --- a/src/ExplicitImports.jl +++ b/src/ExplicitImports.jl @@ -1,9 +1,9 @@ module ExplicitImports using JuliaSyntax, AbstractTrees - # suppress warning about Base.parse collision, even though parse is never used - # this avoids a warning when loading the package while creating an unused explicit import - # the former occurs for all users, the latter only for developers of this package +# suppress warning about Base.parse collision, even though parse is never used +# this avoids a warning when loading the package while creating an unused explicit import +# the former occurs for all users, the latter only for developers of this package using JuliaSyntax: parse using AbstractTrees: parent using TOML: parsefile @@ -53,10 +53,7 @@ include("improper_explicit_imports.jl") include("interactive_usage.jl") include("checks.jl") include("deprecated.jl") - -if isdefined(Base, Symbol("@main")) - include("main.jl") -end +include("main.jl") struct FileNotFoundException <: Exception end diff --git a/src/main.jl b/src/main.jl index c4517e9..9bdb6fb 100644 --- a/src/main.jl +++ b/src/main.jl @@ -1,3 +1,12 @@ +const 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"] +const EXCLUDE_PREFIX = "exclude_" + function err(str) printstyled(stderr, "ERROR: "; bold=true, color=:red) println(stderr, @@ -6,7 +15,7 @@ function err(str) return 1 end -function auto_print_explicit_imports(path) +function get_package_name_from_project_toml(path) if endswith(path, "Project.toml") project_path = path elseif isfile(joinpath(path, "Project.toml")) @@ -19,13 +28,34 @@ function auto_print_explicit_imports(path) return err("`Project.toml` does not have `name` entry; does not correspond to valid Julia package") end package = Symbol(project["name"]) - Base.set_active_project(project_path) + return package, project_path +end + +function activate_and_load(package, project_path) + @static if isdefined(Base, :set_active_project) + Base.set_active_project(project_path) + else + @eval Main begin + using Pkg + Pkg.activate($project_path) + end + end @eval Main begin using $package: $package - using ExplicitImports: print_explicit_imports + using ExplicitImports: ExplicitImports end - @eval Main begin - print_explicit_imports($package) +end + +function run_checks(package, selected_checks) + for check in selected_checks + @info "Checking $check" + try + @eval Main ExplicitImports.$(Symbol("check_" * check))($package) + catch e + printstyled(stderr, "ERROR: "; bold=true, color=:red) + Base.showerror(stderr, e) + return 1 + end end return 0 end @@ -37,7 +67,7 @@ function print_help() println(io, " ExplicitImports.main - analyze a package's namespace") println(io) printstyled(io, "SYNOPSIS\n"; bold=true) - println(io, " julia -m ExplicitImports ") + println(io, " julia -m ExplicitImports [OPTIONS] ") println(io) printstyled(io, "DESCRIPTION\n"; bold=true) println(io, @@ -49,12 +79,37 @@ function print_help() println(io, " Path to the root directory of the package (default: pwd)") println(io, " --help") println(io, " Show this message") + println(io, " --check") + println(io, + " Run checks instead of printing. If --checklist is not specified, all checks are run") + println(io, " --checklist ,...") + println(io, + """ Run checks specified by ,,... + This will imply --check. + + Valid values for each check are: + - Individual checks: + $(join(CHECKS, ",\n\t\t ")) + - Select all checks: all + - Exclude a check: prepend an individual check with '$EXCLUDE_PREFIX' + + 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. + """) return end -function (@main)(args) +function main(args) # Argument defaults path::String = pwd() + valid_check_values = [CHECKS; "all"; EXCLUDE_PREFIX .* CHECKS] + selected_checks = copy(CHECKS) + should_run_checks = false + should_print = false + path = "." + # Argument parsing while length(args) > 0 x = popfirst!(args) @@ -62,13 +117,82 @@ function (@main)(args) # Print help and return (even if other arguments are present) print_help() return 0 - elseif length(args) == 0 && isdir(abspath(x)) || isfile(abspath(x)) - # If args is empty and the argument is a directory this is the root directory - path = abspath(x) + elseif x == "--check" + should_run_checks = true + elseif x == "--print" + should_print = true + elseif x == "--checklist" + should_run_checks = true # Automatically imply --check + if length(args) == 0 + return err("Argument `--checklist` requires a value") + end + values = split(popfirst!(args), ",") + # If any of passed checks is not an exclude, then starts with an empty list + if any(.!startswith(EXCLUDE_PREFIX).(values)) + selected_checks = String[] + end + for value in values + unique!(selected_checks) + if !(value in valid_check_values) + return err("Invalid check passed to --checklist: $value") + end + if value == "all" + selected_checks = copy(CHECKS) + elseif value in checks + push!(selected_checks, value) + elseif startswith(EXCLUDE_PREFIX)(value) + check = value[(1 + length(EXCLUDE_PREFIX)):end] + if !(check in CHECKS) + return err("Check $check is not part of the valid checks, so it can't be excluded") + end + i = findfirst(selected_checks .== check) + if !isnothing(i) + deleteat!(selected_checks, i) + end + end + end else - # Unknown argument - return err("Argument `$x` is not a supported flag, directory, or file.") + # The path might be out of order + if isdir(abspath(x)) || isfile(abspath(x)) + # If the argument is a directory this is the root directory + path = abspath(x) + else + # Unknown argument + return err("Argument `$x` is not a supported flag, directory, or file. See the output of `--help` for usage details") + end end end - return ExplicitImports.auto_print_explicit_imports(path) + + # Print by default + if !should_run_checks && !should_print + should_print = true + end + + ret = get_package_name_from_project_toml(path) + if ret isa Integer # handle errors + return ret + end + package, project_path = ret + + activate_and_load(package, project_path) + if should_print + try + @eval Main ExplicitImports.print_explicit_imports($package) + catch e + printstyled(stderr, "ERROR: "; bold=true, color=:red) + Base.showerror(stderr, e) + return 1 + end + end + if should_run_checks + if length(selected_checks) == 0 + return err("The passed combination of checks $values made the selection empty") + end + return run_checks(package, selected_checks) + end + return 0 +end + +@static if isdefined(Base, Symbol("@main")) + @main end diff --git a/test/main.jl b/test/main.jl index 1e353ce..aee6407 100644 --- a/test/main.jl +++ b/test/main.jl @@ -1,18 +1,46 @@ -# Here we test the `julia -m ExplicitImports` functionality -cmd = Base.julia_cmd() -dir = pkgdir(ExplicitImports) -help = replace(readchomp(`$cmd --project=$(dir) -m ExplicitImports --help`), r"\s+" => " ") -@test contains(help, "SYNOPSIS") -@test contains(help, "Path to the root directory") -run1 = replace(readchomp(`$cmd --project=$(dir) -m ExplicitImports $dir`), r"\s+" => " ") -@test contains(run1, "These could be explicitly imported as follows") -run2 = replace(readchomp(`$cmd --project=$(dir) -m ExplicitImports $dir/Project.toml`), - r"\s+" => " ") -@test contains(run2, "These could be explicitly imported as follows") -io = IOBuffer() -err_run = success(pipeline(`$cmd --project=$(dir) -m ExplicitImports $dir/blah.toml`; - stderr=io)) -@test !err_run -str = replace(String(take!(io)), r"\s+" => " ") -@test contains(str, - "is not a supported flag, directory, or file. See the output of `--help` for usage details") +# We need both `@main` and `julia -m` to be supported: +if isdefined(Base, Symbol("@main")) && VERSION >= v"1.12.0-DEV.102" + @testset "Test `julia -m ExplicitImports` functionality" begin + cmd = Base.julia_cmd() + dir = pkgdir(ExplicitImports) + help = replace(readchomp(`$cmd --project=$(dir) -m ExplicitImports --help`), + r"\s+" => " ") + @test contains(help, "SYNOPSIS") + @test contains(help, "Path to the root directory") + run1 = replace(readchomp(`$cmd --project=$(dir) -m ExplicitImports $dir`), + r"\s+" => " ") + @test contains(run1, "These could be explicitly imported as follows") + run2 = replace(readchomp(`$cmd --project=$(dir) -m ExplicitImports $dir/Project.toml`), + r"\s+" => " ") + @test contains(run2, "These could be explicitly imported as follows") + io = IOBuffer() + err_run = success(pipeline(`$cmd --project=$(dir) -m ExplicitImports $dir/blah.toml`; + stderr=io)) + @test !err_run + str = replace(String(take!(io)), r"\s+" => " ") + @test contains(str, + "is not a supported flag, directory, or file. See the output of `--help` for usage details") + end +end + +@testset "Test main functionality" begin + cmd = Base.julia_cmd() + dir = pkgdir(ExplicitImports) + help = replace(readchomp(`$cmd --project=$(dir) -e 'using ExplicitImports: main; exit(main(["--help"]))'`), + r"\s+" => " ") + @test contains(help, "SYNOPSIS") + @test contains(help, "Path to the root directory") + run1 = replace(readchomp(`$cmd --project=$(dir) -e "using ExplicitImports: main; exit(main([\"$(dir)\"]))"`), + r"\s+" => " ") + @test contains(run1, "These could be explicitly imported as follows") + run2 = replace(readchomp(`$cmd --project=$(dir) -e "using ExplicitImports: main; exit(main([\"$(dir)/Project.toml\"]))"`), + r"\s+" => " ") + @test contains(run2, "These could be explicitly imported as follows") + io = IOBuffer() + err_run = success(pipeline(`$cmd --project=$(dir) -e "using ExplicitImports: main; exit(main([\"$(dir)/blah.toml\"]))"`; + stderr=io)) + @test !err_run + str = replace(String(take!(io)), r"\s+" => " ") + @test contains(str, + "is not a supported flag, directory, or file. See the output of `--help` for usage details") +end diff --git a/test/runtests.jl b/test/runtests.jl index c81dda6..ed180b4 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -71,11 +71,7 @@ include("script.jl") include("imports.jl") include("test_qualified_access.jl") include("test_explicit_imports.jl") - -# We need both `@main` and `julia -m` to be supported: -if isdefined(Base, Symbol("@main")) && VERSION >= v"1.12.0-DEV.102" - include("main.jl") -end +include("main.jl") # For deprecations, we are using `maxlog`, which # the TestLogger only respects in Julia 1.8+.