Handle invalid syntax in doctests (#1062)

* Handle invalid syntax in doctests

* Pass empty backtrace for parse errors

In the terminal, parse errors do not have a backtrace. But we still need
to initialize the result.bt field.

* Handle backtrace more properly

The heuristic in error_to_string which removes the backtrace below the
highest Core.eval call works in most cases but will fail if
makedocs/doctest gets called in a more complicated context, as there may
sometimes be eval calls higher up in the stacktrace.

CHANGELOG.md

@@ -30,6 +30,10 @@
 
 * ![Bugfix][badge-bugfix] `makedocs` now throws an error when the format objects (`Documenter.HTML`, `LaTeX`, `Markdown`) get passed positionally. The format types are no longer subtypes of `Documenter.Plugin`. ([#1046][github-1046], [#1061][github-1061])
 
+* ![Bugfix][badge-bugfix] Doctesting now also handles doctests that contain invalid syntax and throw parsing errors. ([#487][github-487], [#1062][github-1062])
+
+* ![Bugfix][badge-bugfix] Stacktraces in doctests that throw an error are now filtered more thoroughly, fixing an issue where too much of the stacktrace was included when `doctest` or `makedocs` was called from a more complicated context. ([#1062][github-1062])
+
 * ![Experimental][badge-experimental] ![Feature][badge-feature] The current working directory when evaluating `@repl` and `@example` blocks can now be set to a fixed directory by passing the `workdir` keyword to `makedocs`. _The new keyword and its behaviour are experimental and not part of the public API._ ([#1013][github-1013], [#1025][github-1025])
 
 ## Version `v0.22.5`
@@ -290,6 +294,7 @@
 * ![Bugfix][badge-bugfix] At-docs blocks no longer give an error when containing empty lines. ([#823][github-823], [#824][github-824])
 
 [github-198]: https://github.com/JuliaDocs/Documenter.jl/issues/198
+[github-487]: https://github.com/JuliaDocs/Documenter.jl/issues/487
 [github-511]: https://github.com/JuliaDocs/Documenter.jl/pull/511
 [github-535]: https://github.com/JuliaDocs/Documenter.jl/issues/535
 [github-697]: https://github.com/JuliaDocs/Documenter.jl/pull/697
@@ -369,6 +374,7 @@
 [github-1047]: https://github.com/JuliaDocs/Documenter.jl/pull/1047
 [github-1054]: https://github.com/JuliaDocs/Documenter.jl/pull/1054
 [github-1061]: https://github.com/JuliaDocs/Documenter.jl/pull/1061
+[github-1062]: https://github.com/JuliaDocs/Documenter.jl/pull/1062
 
 [documenterlatex]: https://github.com/JuliaDocs/DocumenterLaTeX.jl
 [documentermarkdown]: https://github.com/JuliaDocs/DocumenterMarkdown.jl

src/DocTests.jl

@@ -200,7 +200,7 @@ end
 function eval_repl(block, sandbox, meta::Dict, doc::Documents.Document, page)
     for (input, output) in repl_splitter(block.code)
         result = Result(block, input, output, meta[:CurrentFile])
-        for (ex, str) in Utilities.parseblock(input, doc, page; keywords = false)
+        for (ex, str) in Utilities.parseblock(input, doc, page; keywords = false, raise=false)
             # Input containing a semi-colon gets suppressed in the final output.
             result.hide = REPL.ends_with_semicolon(str)
             (value, success, backtrace, text) = Utilities.withoutput() do
@@ -212,6 +212,8 @@ function eval_repl(block, sandbox, meta::Dict, doc::Documents.Document, page)
             if !success
                 result.bt = backtrace
             end
+            # don't evaluate further if there is a parse error
+            isa(ex, Expr) && ex.head === :error && break
         end
         checkresult(sandbox, result, meta, doc)
     end
@@ -227,7 +229,7 @@ function eval_script(block, sandbox, meta::Dict, doc::Documents.Document, page)
     input  = rstrip(input, '\n')
     output = lstrip(output, '\n')
     result = Result(block, input, output, meta[:CurrentFile])
-    for (ex, str) in Utilities.parseblock(input, doc, page; keywords = false)
+    for (ex, str) in Utilities.parseblock(input, doc, page; keywords = false, raise=false)
         (value, success, backtrace, text) = Utilities.withoutput() do
             Core.eval(sandbox, ex)
         end
@@ -309,14 +311,34 @@ function result_to_string(buf, value)
 end
 
 function error_to_string(buf, er, bt)
-    # Remove unimportant backtrace info.
+    # Remove unimportant backtrace info. TODO: this backtrace handling should maybe be done
+    # by Utilities.withoutput() already.
+    bt = remove_common_backtrace(bt, backtrace())
+    # Remove everything below the last eval call (which should be the one in withoutput)
     index = findlast(ptr -> Base.ip_matches_func(ptr, :eval), bt)
+    bt = (index === nothing) ? bt : bt[1:(index - 1)]
     # Print a REPL-like error message.
     print(buf, "ERROR: ")
-    Base.invokelatest(showerror, buf, er, index === nothing ? bt : bt[1:(index - 1)])
+    Base.invokelatest(showerror, buf, er, bt)
     return sanitise(buf)
 end
 
+function remove_common_backtrace(bt, reference_bt)
+    cutoff = nothing
+    # We'll start from the top of the backtrace (end of the array) and go down, checking
+    # if the backtraces agree
+    for ridx in 1:length(bt)
+        # Cancel search if we run out the reference BT or find a non-matching one frames:
+        if ridx > length(reference_bt) || bt[length(bt) - ridx + 1] != reference_bt[length(reference_bt) - ridx + 1]
+            cutoff = length(bt) - ridx + 1
+            break
+        end
+    end
+    # It's possible that the loop does not find anything, i.e. that all BT elements are in
+    # the reference_BT too. In that case we'll just return an empty BT.
+    bt[1:(cutoff === nothing ? 0 : cutoff)]
+end
+
 # Strip trailing whitespace from each line and return resulting string
 function sanitise(buffer)
     out = IOBuffer()

src/Documenter.jl

@@ -907,8 +907,9 @@ function doctest(
                 modules = modules,
             )
             true
-        catch e
-            @error "Doctesting failed" e
+        catch err
+            @error "Doctesting failed"
+            showerror(stdout, err, catch_backtrace())
             false
         finally
             rm(dir; recursive=true)

src/Utilities/Utilities.jl

@@ -84,8 +84,12 @@ Returns a `Vector` of tuples `(expr, code)`, where `expr` is the corresponding e
 parsed from.
 
 The keyword argument `skip = N` drops the leading `N` lines from the input string.
+
+If `raise=false` is passed, the `Meta.parse` does not raise an exception on parse errors,
+but instead returns an expression that will raise an error when evaluated. `parseblock`
+returns this expression normally and it must be handled appropriately by the caller.
 """
-function parseblock(code::AbstractString, doc, file; skip = 0, keywords = true)
+function parseblock(code::AbstractString, doc, file; skip = 0, keywords = true, raise=true)
     # Drop `skip` leading lines from the code block. Needed for deprecated `{docs}` syntax.
     code = string(code, '\n')
     code = last(split(code, '\n', limit = skip + 1))
@@ -102,7 +106,7 @@ function parseblock(code::AbstractString, doc, file; skip = 0, keywords = true)
                 (QuoteNode(keyword), cursor + lastindex(line))
             else
                 try
-                    Meta.parse(code, cursor)
+                    Meta.parse(code, cursor; raise=raise)
                 catch err
                     push!(doc.internal.errors, :parse_error)
                     @warn "failed to parse exception in $(Utilities.locrepr(file))" exception = err

test/doctests/doctestapi.jl

@@ -97,6 +97,49 @@ module DocTest5
     end
 end
 
+"""
+```jldoctest
+julia> map(tuple, 1/(i+j) for i=1:2, j=1:2, [1:4;])
+ERROR: syntax: invalid iteration specification
+```
+```jldoctest
+julia> 1.2.3
+ERROR: syntax: invalid numeric constant "1.2."
+```
+```jldoctest
+println(9.8.7)
+# output
+ERROR: syntax: invalid numeric constant "9.8."
+```
+```jldoctest
+julia> Meta.ParseError("foo")
+Base.Meta.ParseError("foo")
+
+julia> Meta.ParseError("foo") |> throw
+ERROR: Base.Meta.ParseError("foo")
+Stacktrace:
+[...]
+```
+"""
+module ParseErrorSuccess end
+
+"""
+```jldoctest
+julia> map(tuple, 1/(i+j) for i=1:2, j=1:2, [1:4;])
+ERROR: syntax: invalid iteration specificationX
+```
+"""
+module ParseErrorFail end
+
+"""
+```jldoctest
+println(9.8.7)
+# output
+ERROR: syntax: invalid numeric constant "1.2."
+```
+"""
+module ScriptParseErrorFail end
+
 @testset "Documenter.doctest" begin
     # DocTest1
     run_doctest(nothing, [DocTest1]) do result, success, backtrace, output
@@ -148,6 +191,20 @@ end
         @test success
         @test result isa Test.DefaultTestSet
     end
+
+    # Parse errors in doctests (https://github.com/JuliaDocs/Documenter.jl/issues/1046)
+    run_doctest(nothing, [ParseErrorSuccess]) do result, success, backtrace, output
+        @test success
+        @test result isa Test.DefaultTestSet
+    end
+    run_doctest(nothing, [ParseErrorFail]) do result, success, backtrace, output
+        @test !success
+        @test result isa TestSetException
+    end
+    run_doctest(nothing, [ScriptParseErrorFail]) do result, success, backtrace, output
+        @test !success
+        @test result isa TestSetException
+    end
 end
 
 end # module

test/doctests/doctests.jl

@@ -213,4 +213,18 @@ rfile(filename) = joinpath(@__DIR__, "stdouts", filename)
     end
 end
 
+using Documenter.DocTests: remove_common_backtrace
+@testset "DocTest.remove_common_backtrace" begin
+    @test remove_common_backtrace([], []) == []
+    @test remove_common_backtrace([1], []) == [1]
+    @test remove_common_backtrace([1,2], []) == [1,2]
+    @test remove_common_backtrace([1,2,3], [1]) == [1,2,3]
+    @test remove_common_backtrace([1,2,3], [2]) == [1,2,3]
+    @test remove_common_backtrace([1,2,3], [3]) == [1,2]
+    @test remove_common_backtrace([1,2,3], [2,3]) == [1]
+    @test remove_common_backtrace([1,2,3], [1,3]) == [1,2]
+    @test remove_common_backtrace([1,2,3], [1,2,3]) == []
+    @test remove_common_backtrace([1,2,3], [0,1,2,3]) == []
+end
+
 end # module