Skip to content

Commit

Permalink
Create script and pre-commit hook
Browse files Browse the repository at this point in the history
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 ericphanson#85
  • Loading branch information
abelsiqueira committed Oct 4, 2024
1 parent e78b1ee commit 9345a73
Show file tree
Hide file tree
Showing 8 changed files with 273 additions and 69 deletions.
6 changes: 6 additions & 0 deletions .pre-commit-hooks.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
- id: explicit-imports
name: ExplicitImports checks
entry: ./scripts/explicit-imports.jl --check
files: ^src.*\.jl$
language: script
pass_filenames: false
92 changes: 92 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,98 @@ 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!

## Pre-commit hook and script usage

One 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 # TODO: Add correct version
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.
The `--checklist` argument allows you to specify which checks to run. If omitted, all checks are run.

The `--print` argument will print the explicit_imports, which might be useful for fixing the issues.
They issues are only shown if the checks fail, or if you run pre-commit with `--verbose`.

This pre-commit hook uses the script in <scripts/explicit-imports.jl>, which runs `ExplicitImports.cli(ARGS)`.
It can be also be run directly via:

```bash
julia <path/to/ExplicitImports.jl>/scripts/explicit-imports.jl --print --checklist exclude_all_qualified_accesses_are_public
```

Or using the internal `cli` function directly:

```bash
julia -e 'using ExplicitImports: cli; cli(["--print", "--checklist", "exclude_all_qualified_accesses_are_public"])'
```

To see all arguments, including the valid options to `--checks`, run either of

```bash
julia <path/to/ExplicitImports.jl>/scripts/explicit-imports.jl --help
julia -e 'using ExplicitImports: cli; cli(["--help"])'
```

which has current output:

```man
NAME
ExplicitImports.main - analyze a package's namespace
SYNOPSIS
julia -m ExplicitImports [OPTIONS] <path>
DESCRIPTION
`ExplicitImports.main` (typically invoked as `julia -m ExplicitImports`)
analyzes a package's imports and qualified accesses, and prints the results.

OPTIONS
<path>
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 <check1,check2>,...
Run checks specified by <check1>,<check2>,...
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 individial checks, it will assume that it starts from an empty list, and then includes.
Passing both individual and exclusion checks does not make sense.
```

### Julia v1.12+

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.

```bash
julia +nightly -m ExplicitImports --print --checklist exclude_all_qualified_accesses_are_public
```

## Limitations

### Some tricky scoping situations are not handled correctly
Expand Down
25 changes: 0 additions & 25 deletions docs/src/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <path>
DESCRIPTION
`ExplicitImports.main` (typically invoked as `julia -m ExplicitImports`)
analyzes a package's imports and qualified accesses, and prints the results.
OPTIONS
<path>
Path to the root directory of the package (default: pwd)
--help
Show this message
```
4 changes: 4 additions & 0 deletions scripts/explicit-imports.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/usr/bin/env julia
using ExplicitImports: cli

cli(ARGS)
11 changes: 4 additions & 7 deletions src/ExplicitImports.jl
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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

Expand Down
134 changes: 120 additions & 14 deletions src/main.jl
Original file line number Diff line number Diff line change
@@ -1,12 +1,21 @@
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,
str,
" See the output of `--help` for usage details.")
return 1
return exit(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"))
Expand All @@ -19,15 +28,28 @@ 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"])
return package, project_path
end

function activate_and_load(package, project_path)
Base.set_active_project(project_path)
@eval Main begin
using $package: $package
using ExplicitImports: print_explicit_imports
using ExplicitImports: ExplicitImports
end
@eval Main begin
print_explicit_imports($package)
end

function auto_print_explicit_imports(package, project_path)
activate_and_load(package, project_path)
@eval Main ExplicitImports.print_explicit_imports($package)
end

function run_checks(package, project_path, selected_checks)
activate_and_load(package, project_path)
for check in selected_checks
@info "Running $check"
@eval Main ExplicitImports.$(Symbol("check_" * check))($package)
end
return 0
end

# Print a typical cli program help message
Expand All @@ -37,7 +59,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 <path>")
println(io, " julia -m ExplicitImports [OPTIONS] <path>")
println(io)
printstyled(io, "DESCRIPTION\n"; bold=true)
println(io,
Expand All @@ -49,26 +71,110 @@ 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 <check1,check2>,...")
println(io,
""" Run checks specified by <check1>,<check2>,...
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 individial 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 cli(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)
if x == "--help"
# 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

package, project_path = get_package_name_from_project_toml(path)

if should_print
ExplicitImports.auto_print_explicit_imports(package, project_path)
end
if should_run_checks
if length(selected_checks) == 0
return err("The passed combination of checks $values made the selection empty")
end
run_checks(package, project_path, selected_checks)
end
end

if isdefined(Base, Symbol("@main"))
function (@main)(args)
return cli(args)
end
end
Loading

0 comments on commit 9345a73

Please sign in to comment.