Add support for "warnings as errors"

Hi everyone, I'm happy to propose a new flag to swift-docc to add support for "warning as errors".

This is an opt-in feature that you use via the following flag (--treat-warning-as-error):

docc convert swift-markdown/Sources/Markdown/Markdown.docc`
--fallback-display-name SwiftDocC
--fallback-bundle-identifier org.swift.SwiftDocC 
--fallback-bundle-version 1.0.0
--additional-symbol-graph-dir xx/swift-markdown.build/Debug/Markdown.build/symbol-graph
--treat-warning-as-error

Below is the description of the inspiration.

Description

Adopters can pass a flag that will promote warnings when building the documentation into errors that fail the doc build.

Motivation

Lots of projects want to build their documentation in CI. In these cases it's possible to introduce docs warnings that provide useful signal, such as notifying us that there is a broken reference in the docs. Catching this in CI allows us to produce the best documentation possible.

Importance

This is fairly important: while it can be retroactively handled by processing the textual output from the build, this is brittle and adds a tricky dependency on the text output format.

See the full description of the issue here Add support for "warnings as errors" · Issue #363 · apple/swift-docc · GitHub @lukasa

And here is a prototype implementation for this Add support for "warnings as errors" by Kyle-Ye · Pull Request #365 · apple/swift-docc · GitHub

The name and the overall behavior is up for discussion.

  • Currently we choose "treat-warning-as-error" for the name
  • And the original warning and error diagnostic will become indistinguishable. Should we keep the original output format?
    eg.
error: Test error
warning: Test warning

Will become the following if we pass the new flag.

error: Test error
error: Test warning
5 Likes

Thanks for putting up this patch! For my part, I think a tweak to the name might be good (treat-warnings-as-errors reads a bit more like English). Beyond that, I think this works great: there's no need to keep the old output format, as we're really just looking for behaviour that's similar to compilers today.

Nice one, thanks again!

2 Likes

Excellent, I think I also requested this somewhere else a while back... :slight_smile:

Two things to consider:

  1. Please make sure how it works with transitive dependencies. We fixed the behavior for warnings as errors in SwiftPM recently so it affects your package, and not packages you depend on - since you have no chance of fixing them after all. The same should be true for this option, make "my" warnings errors, but if I depend on e.g. package X and it has a warning for some reason, this need not break my build.
    This was implemented here: suppress warnings from remote dependencies by tomerd · Pull Request #5605 · apple/swift-package-manager · GitHub and docc warnings should work the same way.

  2. The flag name may want to align more with the compiler flag? We have the warnings-as-errors flag for compiler errors... maybe use the same wording here?

    • How would an invocation look through the swiftpm plugin? Most probably swift package preview-documentation --warnings-as-errors which reads fine, so I'd probably drop the "treat" and make use the same plural form as Swift itself uses for the flag.

Very nice to see this being worked on, I'll be able to get rid of yet another "grep for warnings and fail build" from our builds in distributed actors :+1:

3 Likes

As @ktoso mentioned that there is already a flag named "warnings-as-errors" in swift compiler, maybe we should align with it to use the same flag name - "warnings-as-errors".

1 Like

Yes, I think other packages' documentation warning should not affect the main target. I think we could either just suppress all the diagnostic from other packages or the warnings-as-errors transform should only work for the main target.

If we choose the former, it will be another PR which does not block the current discussed PR.

2 Likes

This all sounds great @Kyle-Ye, thanks for contributing this! I agree on the naming of the argument, we should use the same name as what the Swift compiler uses.

2 Likes