Compiler warnings break standard testing workflow

[adamnfish]

This is a great idea! Sadly, because it emits compiler warnings it breaks my workflow and I suspect my approach is not unique.

TL;DR; this plugin does not support TDD if fatal warnings are enabled (and they should be enabled). I'm going to describe my workflow to highlight what is broken, but I'd observe that this also breaks PDD (println-driven-development™) and I think these two approaches combined probably cover most developers.

Every Scala project should have -Xfatal-warnings enabled. This means things that emit compiler warnings need to be "real" problems, rather than things that are only warnings in a given context. In the case of ??? I would expect this to be a fatal error when building a production jar, but during development (and particularly while tests are compiling and executing) ??? is perfectly normal or indeed even encouraged.

My workflow for Scala code is as follows:

1. Stub functions to see how a part of the program fits together

This means writing the signatures and filling in the implementations with ???. This allows the computer and I to work together designing a program that makes sense before I start spending time on the implementations.

Using this plugin with fatal warnings (and as above, all Scala projects should have fatal warnings enabled) prevents this approach from working. It may be that if the only remaining compiler errors are from this plugin then the program is ready for the next step, but I'd always be nervous that one of these errors is masking something else that would only be revealed after the implementation removes the typed hole warnings.

2. Write unit tests that capture the expected behaviour of the stubbed functions

Doing this first makes sure I understand the problem before I start implementing, encourages higher quality tests by thinking about them separately to the implementation and makes sure the tests are in place while the code evolves to speed implementation (below).

Using this plugin prevents this step from working as well. In this case we can never get as far as running the tests until all the implementations are in place.

3. Evolve the implementation with help from the tests

It's impossible to work on everything at once so here I'd expect to fill in the building bit by bit using the scaffolding of types and tests. It's important the tests are in place before the code so we can see them turn from red to green as we progress.

Of course, the same applies here. We can only run the tests (or indeed, parts of the program) when the whole thing is finished.

What I'd like

This plugin is a fantastic idea, but I think in this form it isn't quite right. I'd say my "ideal dreamland" for this plugin would be that compilation fails when building a production jar (e.g. sbt commands like build, start, package, release), but only warn during tests (test, test*) - even if fatal warnings is enabled. For locally running the program in dev mode (run) I could be convinced either way but I suspect most people would expect compilation to succeed while running a partially built application to support PDD™. This is taking some inspiration from Elm, where the Debug package can be used in testing and development, but will cause a compilation error when building a production application.

I'm not sure how this could be achieved by a single plugin, I'd imagine it would need explicit support from all the plugins that do testing and building, so they'd be able to select the correct mode based on which of their commands is being invoked. I mention it in case I'm missing something, Elm hangs this behaviour off the optimize flag (which it uses when creating prod builds). It's possible there's a corresponding piece of state in sbt that could be used to determine whether ??? should be a real warning that causes compilation to fail (Elm output for a use of the equivalent, Debug.todo is below and here's the docs for it).

Because of all of the above, I don't think this plugin should emit warnings by default, or if it should then it needs to be overridable somehow. If it printed at a message level then the workflow problem would be fixed but we'd lose some of the safety it adds. If this plugin adds a flag then we'd be able to whitelist/blacklist it ourselves to be able to tailor the behaviour to our own needs, particularly if we're using non-standard sbt plugins (warnOnNotImplemented in Release := true / warnOnNotImplemented in (Test, Run) := false, for example).

Sorry for the long issue! I was really excited by the idea, then correspondingly saddened to find I couldn't use it so I put a bit of thought into what it would take to make it work.

Reference

Here's the Elm's output if you try and build an app that contains a TODO, but you can run it locally and run the tests just fine. Elm does not have fancy typed hole messages yet though ;-)

$ elm-app build

Creating an optimized production build...
Failed to compile.

./src/Main.elm
Error: Compiler process exited with error Compilation failed

-- DEBUG REMNANTS --------------------------------------------------------------

There are uses of the `Debug` module in the following modules:

    Main

But the --optimize flag only works if all `Debug` functions are removed!

Note: The issue is that --optimize strips out info needed by `Debug` functions.
Here are two examples:

    (1) It shortens record field names. This makes the generated JavaScript is
    smaller, but `Debug.toString` cannot know the real field names anymore.

    (2) Values like `type Height = Height Float` are unboxed. This reduces
    allocation, but it also means that `Debug.toString` cannot tell if it is
    looking at a `Height` or `Float` value.

There are a few other cases like that, and it will be much worse once we start
inlining code. That optimization could move `Debug.log` and `Debug.todo` calls,
resulting in unpredictable behavior. I hope that clarifies why this restriction
exists!

[cb372]

Hi Adam! 👋

Would this help as a first step? #7

[adamnfish]

Yep, that makes it usable, thanks! :-D

I'd encourage you to canvas opinion to see if it's just me, or if lots of people would prefer this to not block test/dev compilation by default. I'm happy to set a flag if I'm in the minority, thanks for adding the option.

See you at ScalaX!

[cb372]

This is not really an answer to any of your questions, but hopefully provides a bit of context about the motivation for the plugin.

I think the plugin serves a couple of different purposes.

The first is for people who want to treat ??? as a TODO and make sure their code doesn't accidentally build and go to production with TODOs left in it, as you described above.

I didn't really think about this use case when I was making the plugin, but I assume it is the reason for its unexpected popularity.

The second, admittedly more niche, use case is for people who want to treat ??? as a "typed hole" in order to do Type Driven Development (TyDD). In other words their development workflow involves a lot of interaction with the compiler, and they lean heavily on types to guide them towards the correct implementation.

In this kind of workflow a common action is to deliberately leave a hole in the code and then ask the compiler to give them information about the types of both the hole itself and other values in the surrounding context that they might want to use in their implementation.

[adamnfish]

Yep, totally. I think they're complimentary though, especially in a language that lacks syntax for dependent types. I think this plugin could open up Type Driven Development to the (P/T)DD crowd, but only if it supports both workflows.

However, I appreciate you probably want to use the warning mechanism to get the information into tooling (e.g. IDEs) without having to build something bespoke, which is totally valid.