Skip to content

feat!: test imports and use of private APIs through ExplicitImports.jl #369

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gdalle wants to merge 7 commits into
base: master
Choose a base branch
from
Open

feat!: test imports and use of private APIs through ExplicitImports.jl #369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
91513f7
feat!: test explicit imports using ExplicitImports.jl
gdalle Feb 23, 2026
3b1301d
chore: changelog
gdalle Feb 23, 2026
c4c9b1a
chore: bump Julia compat to 1.10 and remove CI below LTS
gdalle Feb 23, 2026
bf3122a
chore: remove noise
gdalle Feb 25, 2026
e6e6c42
chore: remove more noise
gdalle Feb 25, 2026
52eadb1
fix: skip explicit imports test on 1.10 and 1.11
gdalle Feb 25, 2026
bcabf1d
Merge branch 'master' into gd/explicit
gdalle Feb 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,10 @@ jobs:
- '1.12-nightly'
- '1.11'
- '1.10'
- '1.9'
- '1.8'
- '1.7'
- '1.6'
- 'nightly'
include:
- os: windows-latest
julia-version: '1'
- os: windows-latest
julia-version: '1.6'
- os: windows-latest
julia-version: '1.12-nightly'
- os: windows-latest
Expand Down
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed

- The minimum supported julia version is increased to 1.6. ([#328])
- Explicit imports are tested using ExplicitImports.jl ([#369])

## Version [v0.8.14] - 2025-08-04

Expand Down Expand Up @@ -76,7 +77,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Use [Changelog.jl](https://github.com/JuliaDocs/Changelog.jl) to generate the changelog, and add it to the documentation. ([#277], [#279])
- `test_project_extras` prints failures the same on all julia versions. In particular, 1.11 and nightly are no outliers. ([#275])


## Version [v0.8.4] - 2023-12-01

### Added
Expand Down
6 changes: 5 additions & 1 deletion Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,17 @@ authors = ["Takafumi Arakaki <aka.tkf@gmail.com> and contributors"]
version = "1.0.0-DEV"

[deps]
ExplicitImports = "7d51a73a-1435-4ff3-83d9-f097790105c7"
Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
TOML = "fa267f1f-6049-4f14-aa54-33bafae1ed76"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

[compat]
ExplicitImports = "1.15.0"
Pkg = "1.6"
TOML = "1.0.3"
Test = "<0.0.1, 1"
julia = "1.6"
julia = "1.10"

[extras]
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
Expand Down
2 changes: 2 additions & 0 deletions docs/Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
[deps]
Changelog = "5217a498-cd5d-4ec6-b8c2-9b85a09b6e3e"
Documenter = "e30172f5-a6a5-5a46-863b-614d45cd2de4"
DocumenterInterLinks = "d12716ef-a0f6-4df4-a9f1-a5a34e75c656"

[compat]
Changelog = "1"
Documenter = "1"
DocumenterInterLinks = "1.1.0"
7 changes: 7 additions & 0 deletions docs/make.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Documenter, Aqua, Changelog
using DocumenterInterLinks: InterLinks

# Generate a Documenter-friendly changelog from CHANGELOG.md
Changelog.generate(
Expand All @@ -8,6 +9,10 @@ Changelog.generate(
repo = "JuliaTesting/Aqua.jl",
)

links = InterLinks(
"ExplicitImports" => "https://juliatesting.github.io/ExplicitImports.jl/stable/",
)

Copy link
Member

@fingolfin fingolfin Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this needed for / what does it gain us?

Copy link
Contributor Author

@gdalle gdalle Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It allows for a direct link to the docstring of ExplicitImports.test_explicit_imports, so that we don't have to separately maintain a list of its keyword arguments

makedocs(;
sitename = "Aqua.jl",
format = Documenter.HTML(;
Expand All @@ -30,9 +35,11 @@ makedocs(;
"piracies.md",
"persistent_tasks.md",
"undocumented_names.md",
"explicit_imports.md",
],
"release-notes.md",
],
plugins = [links],
)

deploydocs(; repo = "github.com/JuliaTesting/Aqua.jl", push_preview = true)
7 changes: 7 additions & 0 deletions docs/src/explicit_imports.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Explicit imports

## [Test function](@id test_explicit_imports)

```@docs
Aqua.test_explicit_imports
```
18 changes: 15 additions & 3 deletions src/Aqua.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
module Aqua

using Base: Docs, PkgId, UUID
using Pkg: Pkg, TOML, PackageSpec
using Pkg.Types: VersionSpec, semver_spec
using Test
using Pkg: Pkg, PackageSpec
using Pkg.Types: VersionSpec
using Pkg.Versions: semver_spec
using Test: Test, @test, @test_broken, @testset, detect_ambiguities
using TOML: TOML

using ExplicitImports: ExplicitImports

include("utils.jl")
include("ambiguities.jl")
Expand All @@ -16,6 +19,7 @@ include("deps_compat.jl")
include("piracies.jl")
include("persistent_tasks.jl")
include("undocumented_names.jl")
include("explicit_imports.jl")

"""
test_all(testtarget::Module)
Expand All @@ -31,6 +35,7 @@ Run the following tests on the module `testtarget`:
* [`test_piracies(testtarget)`](@ref test_piracies)
* [`test_persistent_tasks(testtarget)`](@ref test_persistent_tasks)
* [`test_undocumented_names(testtarget)`](@ref test_undocumented_names)
* [`test_explicit_imports(testtarget)`](@ref test_explicit_imports)

The keyword argument `\$x` (e.g., `ambiguities`) can be used to
control whether or not to run `test_\$x` (e.g., `test_ambiguities`).
Expand All @@ -47,6 +52,7 @@ passed to `\$x` to specify the keyword arguments for `test_\$x`.
- `piracies = true`
- `persistent_tasks = true`
- `undocumented_names = false`
- `explicit_imports = true`
"""
function test_all(
testtarget::Module;
Expand All @@ -59,6 +65,7 @@ function test_all(
piracies = true,
persistent_tasks = true,
undocumented_names = false,
explicit_imports = false,
)
if ambiguities !== false
@testset "Method ambiguity" begin
Expand Down Expand Up @@ -108,6 +115,11 @@ function test_all(
test_undocumented_names(testtarget; askwargs(undocumented_names)...)
end
end
if explicit_imports !== false
@testset "Explicit imports" begin
test_explicit_imports(testtarget; askwargs(explicit_imports)...)
end
end
end

end # module
13 changes: 13 additions & 0 deletions src/explicit_imports.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
"""
test_explicit_imports(m::Module; kwargs...)

Run the various tests provided by the package [ExplicitImports.jl](https://github.com/ericphanson/ExplicitImports.jl).
Copy link
Member

@fingolfin fingolfin Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand: if ExplicitImports.test_explicit_imports already exists, why do we need to add this to Aqua at all?

Copy link
Contributor Author

@gdalle gdalle Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To have everything in one place instead of requiring users to combine several packages when testing code quality

Copy link
Member

@fingolfin fingolfin Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we are missing JET report_package, SnoopeCompile invalidation tests, Runic (or JuliaFormatter???) code formatting.

Sarcasm aside, I don't agree with this being a useful goal.

Copy link
Contributor Author

@gdalle gdalle Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You think this is sarcasm but I actually think these would be other legitimate additions to Aqua

Copy link
Contributor Author

@gdalle gdalle Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main question is what the community wants, because you and I are obviously on opposite sides ^^

Copy link
Member

@fingolfin fingolfin Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main question is what the community wants

Is it, though? It sure is useful what "the community" wants, but it is also exceedingly difficult to actually find that out.

In particular don't find 15 likes on an issue convincing: first off, those are not really high numbers (relative to the size of the Julia community). Moreover you don't see dislikes / don't care / "am afraid of this". And finally I am not convinced that this actually means something (it is not uncommon for people to "like" something in the abstract but then not "like" it in practice once it is there)

But most importantly to me, the set of people who even see these discussions, and comments/likes, is highly self-selected: People who see it are likely to already be interested in enforcing standards, of dealing with the resulting work, they might even read the release notes for Aqua and opt-in to new checks. (Plus: one can only "like" in Discourse, there is no way to "dislikes"; and 7 or 15 are also not very impressive numbers to start with)

But in my work and daily interactions with people who need to work with Julia code and packages but just want to get stuff done, I see a very different picture. There Julia and package updates are seen as something to dread, as something that causes constant churn by introducing breaking changes. I am glad when I can convince these people to use Aqua at all. Each new test is a new hurdle that needs to be carefully weighed: how important is this test, how easy is it to understand what is even about (and why it matters) how hard is it to make it pass (other than turning it off)?

In my subjective view, adding ExplicitImports here is a step to far. No offense intended: I think it's wonderful this package exists, and I am sure it is beneficial for many people. I also think we should mention it (and some other tools and "best practices", such as JET) in the README and manual.

And JET, SnoopCompile etc. are all nice tools in theory, but like the "problems" this PR "revealed" in Aqua itself, they often are very tricky to resolve and require convincing other package authors and even Julia itself to make changes.

Some people love spending their time with that.

Others need to get their job done first.

Again, this is a tricky balance, and admittedly where the cutoff is is subjective. For me one rule of thumb is that stuff were a fix often requires changes outside of your own package is too far.

There is also the classic dependency problem: by tying Aqua to ExplicitImports we also become beholden to decisions you make: if ExplicitImports decides to make a minor release with a stricter check, then every Aqua user is exposed to that, and Aqua will receive the issues complaining about that.

If you really want a "one stop solution"), I suggest to make a new HardcoreAqua package (well, with a better name), which uses Aqua and ExplicitImports, but and if you like alsos enforce isempty(JET.get_reports(JET.report_package(YourPackage))) and run some SnoopCompile checks; and runs Runic to verify code formatting, and whatever you like.

Copy link
Member

@ericphanson ericphanson Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Aqua has a pretty general scope:

Auto QUality Assurance for Julia packages

There's nothing that actually unifies the particular set of checks it does under one theme, they are just a bunch of useful checks. So to me it seems weird to say whatever checks already exist are OK, but other checks are not. And test_all is inherently a one-stop-shop thing, it's run all the checks you can.

Here's a random idea:

  • deprecate test_all
  • introduce test_all_v1 which maps to the existing set of checks and will never get new checks nor get deprecated
  • introduce test_all_v2 which adds ExplicitImports' checks
  • future check additions do not need to be a breaking release of Aqua, they are instead a new function

So anyone can easily stick with test_all_v1 forever, but also can keep up with compat and bugfixes (don't need to be stuck on an old version of Aqua). Then if they want to opt-in to more checks, they can go to v2.

Could also have Aqua.latest_check_version() = 2 so users can get test failures if they aren't on the latest version, if they want, or introduce test_all_latest.

Copy link

@j-fu j-fu Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of "versions" we could call these "levels of strictness". Like e.g. test_all(;strictnesslevel=1). Or make a type out of it: test_all(StrictnessLevel(1)) which then allows to have test_all(StrictnessLevelHighest()). Or what ever else makes s good API.


# Keyword Arguments

Same as those of the function [`ExplicitImports.test_explicit_imports`](@extref).
"""
function test_explicit_imports(m::Module; kwargs...)
# TODO: explicitly list kwargs here?
ExplicitImports.test_explicit_imports(m; kwargs...)
end
54 changes: 53 additions & 1 deletion test/test_smoke.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,57 @@ module TestSmoke
using Aqua

# test defaults
Aqua.test_all(Aqua)

# TODO: fix instead of ignoring
private_explicit_imports_aqua = (:PkgId,) # Base
private_explicit_imports_aquapiracy = (:is_in_mods,) # Test
private_qualified_accesses_aqua = (
:JLOptions, # Base
:PkgId, # Base
:SIGKILL, # Base
:error_color, # Base
:eval, # Base
:isbindingresolved, # Base
:isdeprecated, # Base
:julia_cmd, # Base
:kwcall, # Core
:kwftype, # Core
:load_path_setup_code, # Base
:morespecific, # Base
:require, # Base
:respect_sysimage_versions, # Pkg
:show_tuple_as_call, # Base
:unwrap_unionall, # Base,
)
private_qualified_accesses_aquapiracy = (
:MethodTable, # Core
:TypeofVararg, # Core
:loaded_modules_array, # Base
:methodtable, # Core
:uniontypes, # Base
:visit, # Base
)

explicit_imports_param_for_aqua = @static if VERSION < v"1.12"
false # some symbols were only made public in more recent versions of Julia, the list above is for 1.12
else
(;
all_explicit_imports_are_public = (;
ignore = (
private_explicit_imports_aqua...,
private_explicit_imports_aquapiracy...,
)
),
all_qualified_accesses_are_public = (;
ignore = (
private_qualified_accesses_aqua...,
private_qualified_accesses_aquapiracy...,
)
),
)
end

Aqua.test_all(Aqua; explicit_imports = explicit_imports_param_for_aqua)

# test everything else
Aqua.test_all(
Expand All @@ -16,6 +66,8 @@ Aqua.test_all(
deps_compat = false,
piracies = false,
persistent_tasks = false,
undocumented_names = false,
explicit_imports = false,
)

end # module
Loading