Add test_project_toml_formatting (#34)

* Add test_project_toml_formatting

* Optionally run test_project_toml_formatting via test_all

README.md

@@ -18,6 +18,7 @@ Aqua.jl provides functions to run a few automatable checks for Julia packages:
   (`test/Project.toml`) are consistent (optional).
 * Check that all external packages listed in `deps` have corresponding
   `compat` entry (optional).
+* `Project.toml` formatting is compatible with Pkg.jl output (optional).
 
 ## Quick usage
 

src/Aqua.jl

@@ -16,6 +16,7 @@ include("exports.jl")
 include("project_extras.jl")
 include("stale_deps.jl")
 include("deps_compat.jl")
+include("project_toml_formatting.jl")
 
 """
     test_all(testtarget::Module)
@@ -34,6 +35,7 @@ Run following tests in isolated testset:
 * [`test_project_extras(testtarget)`](@ref test_project_extras) (optional)
 * [`test_stale_deps(testtarget)`](@ref test_stale_deps) (optional)
 * [`test_deps_compat(testtarget)`](@ref test_deps_compat) (optional)
+* [`test_project_toml_formatting(testtarget)`](@ref test_project_toml_formatting) (optional)
 
 The keyword argument `\$x` (e.g., `ambiguities`) can be used to
 control whether or not to run `test_\$x` (e.g., `test_ambiguities`).
@@ -47,6 +49,7 @@ passed to `\$x` to specify the keyword arguments for `test_\$x`.
 - `project_extras = false`
 - `stale_deps = false`
 - `deps_compat = false`
+- `project_toml_formatting = false`
 """
 function test_all(
     testtarget::Module;
@@ -56,6 +59,7 @@ function test_all(
     project_extras = false,
     stale_deps = false,
     deps_compat = false,
+    project_toml_formatting = false,
 )
     @testset "Method ambiguity" begin
         if ambiguities !== false
@@ -92,6 +96,11 @@ function test_all(
             test_deps_compat(testtarget; askwargs(deps_compat)...)
         end
     end
+    @testset "Project.toml formatting" begin
+        if project_toml_formatting !== false
+            test_project_toml_formatting(testtarget; askwargs(project_toml_formatting)...)
+        end
+    end
 end
 
 end # module

src/project_toml_formatting.jl

@@ -0,0 +1,70 @@
+"""
+    Aqua.test_project_toml_formatting(packages)
+"""
+function test_project_toml_formatting(packages)
+    @testset "$(result.label)" for result in analyze_project_toml_formatting(packages)
+        @debug result.label result
+        @test result ⊜ true
+    end
+end
+
+analyze_project_toml_formatting(packages) =
+    [_analyze_project_toml_formatting_1(path) for path in project_toml_files_in(packages)]
+
+project_toml_files_in(path::AbstractString) = [path]
+project_toml_files_in(m::Module) = project_toml_files_in(PkgId(m))
+function project_toml_files_in(pkg::PkgId)
+    srcpath = Base.locate_package(pkg)
+    if srcpath === nothing
+        # TODO: record this as a test failure?
+        error("Package file and directory not found: $pkg")
+    end
+    dir = dirname(dirname(srcpath))
+    paths = [project_toml_path(dir)[1]]
+    p, found = project_toml_path(joinpath(dir, "test"))
+    found && push!(paths, p)
+    return paths
+end
+
+project_toml_files_in(iterable) =
+    [path for x in iterable for path in project_toml_files_in(x)]
+
+function _analyze_project_toml_formatting_1(path::AbstractString)
+    label = path
+
+    if !isfile(path)
+        return LazyTestResult(label, "Path `$path` is not an existing file.", false)
+    end
+
+    original = read(path, String)
+    return _analyze_project_toml_formatting_2(path, original)
+end
+
+function _analyze_project_toml_formatting_2(path::AbstractString, original)
+    @debug "Checking TOML style: `$path`" Text(original)
+    label = path
+
+    prj = TOML.parse(original)
+    formatted = sprint(print_project, prj)
+    if original == formatted
+        LazyTestResult(
+            label,
+            "Running `Pkg.resolve` on `$(path)` did not change the content.",
+            true,
+        )
+    else
+        diff = format_diff(
+            "Original $(basename(path))" => original,
+            "Pkg's output" => formatted,
+        )
+        LazyTestResult(
+            label,
+            """
+            Running `Pkg.resolve` on `$(path)` will change the content.
+
+            $diff
+            """,
+            false,
+        )
+    end
+end

src/utils.jl

@@ -94,3 +94,53 @@ catch
         Pkg.Types.gather_stdlib_uuids  # julia < 1.1
     end
 end
+
+const _project_key_order = ["name", "uuid", "keywords", "license", "desc", "deps", "compat"]
+project_key_order(key::String) =
+    something(findfirst(x -> x == key, _project_key_order), length(_project_key_order) + 1)
+
+print_project(io, dict) =
+    TOML.print(io, dict, sorted = true, by = key -> (project_key_order(key), key))
+
+ensure_exception(e::Exception) = e
+ensure_exception(x) = ErrorException(string(x))
+
+"""
+    trydiff(label_a => text_a, label_b => text_b) -> string or exception
+"""
+function trydiff(
+    (label_a, text_a)::Pair{<:AbstractString,<:AbstractString},
+    (label_b, text_b)::Pair{<:AbstractString,<:AbstractString},
+)
+    # TODO: use a pure-Julia function
+    cmd = `diff --label $label_a --label $label_b -u`
+    mktemp() do path_a, io_a
+        print(io_a, text_a)
+        close(io_a)
+        mktemp() do path_b, io_b
+            print(io_b, text_b)
+            close(io_b)
+            try
+                return read(ignorestatus(`$cmd $path_a $path_b`), String)
+            catch err
+                return ensure_exception(err)
+            end
+        end
+    end
+end
+
+function format_diff(
+    (label_a, text_a)::Pair{<:AbstractString,<:AbstractString},
+    (label_b, text_b)::Pair{<:AbstractString,<:AbstractString},
+)
+    diff = trydiff(label_a => text_a, label_b => text_b)
+    diff isa AbstractString && return diff
+    # Fallback:
+    return """
+    *** $label_a ***
+    $text_a
+
+    *** $label_b ***
+    $text_b
+    """
+end

test/test_project_toml_formatting.jl

@@ -0,0 +1,71 @@
+module TestProjectTomlFormatting
+
+using Aqua: _analyze_project_toml_formatting_2, ⊜
+using Test
+
+@testset "_analyze_project_toml_formatting_2" begin
+    path = "DUMMY/PATH"
+    @testset "pass" begin
+        @test _analyze_project_toml_formatting_2(
+            path,
+            """
+            [deps]
+            Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
+            Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
+            """,
+        ) ⊜ true
+        @test _analyze_project_toml_formatting_2(
+            path,
+            """
+            name = "Aqua"
+            uuid = "4c88cf16-eb10-579e-8560-4a9242c79595"
+            authors = ["Takafumi Arakaki <[email protected]>"]
+            version = "0.4.7-DEV"
+
+            [deps]
+            Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
+            Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
+
+            [compat]
+            julia = "1.0"
+
+            [extras]
+            Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
+
+            [targets]
+            test = ["Test"]
+            """,
+        ) ⊜ true
+    end
+    @testset "failure: reversed deps" begin
+        t = _analyze_project_toml_formatting_2(
+            path,
+            """
+            [deps]
+            Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
+            Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
+            """,
+        )
+        @debug "failure: reversed deps" t
+        @test t ⊜ false
+        @test occursin("change the content", string(t))
+    end
+    @testset "failure: reversed table" begin
+        t = _analyze_project_toml_formatting_2(
+            path,
+            """
+            [compat]
+            julia = "1.0"
+
+            [deps]
+            Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
+            Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
+            """,
+        )
+        @debug "failure: reversed table" t
+        @test t ⊜ false
+        @test occursin("change the content", string(t))
+    end
+end
+
+end  # module

test/test_smoke.jl

@@ -9,6 +9,7 @@ Aqua.test_all(
     undefined_exports = false,
     stale_deps = true,
     deps_compat = true,
+    project_toml_formatting = true,
 )
 
 end  # module

test/test_utils.jl

@@ -1,6 +1,6 @@
 module TestUtils
 
-using Aqua: askwargs
+using Aqua: askwargs, format_diff
 using Test
 
 @testset "askwargs" begin
@@ -10,4 +10,22 @@ using Test
     @test askwargs((a = 1,)) === (a = 1,)
 end
 
+@testset "format_diff" begin
+    @testset "normal" begin
+        if Sys.which("diff") === nothing
+            @info "Comamnd `diff` not found; skip testing `format_diff`."
+        else
+            diff = format_diff("LABEL_A" => "TEXT_A", "LABEL_B" => "TEXT_B")
+            @test occursin("--- LABEL_A", diff)
+            @test occursin("+++ LABEL_B", diff)
+        end
+    end
+    @testset "fallback" begin
+        diff = withenv("PATH" => "/") do
+            format_diff("LABEL_A" => "TEXT_A", "LABEL_B" => "TEXT_B")
+        end
+        @test occursin("*** LABEL_A ***", diff)
+    end
+end
+
 end  # module