Beyond Code Style

They say “with great power comes great responsibility.” Because of the IntelliJ Scala plugin’s huge user base, most default settings tend to become de-facto standards in the Scala community, so we strive to choose the defaults wisely. Moreover, we believe that sometimes such kind of decisions are worthy of detailed explanations, and subject to approval by the community.

As experience has shown, many introduced novelties are often misunderstood and criticized in the beginning. For example, method signature enforcement was initially touted as some quirk that only keeps getting in the way. But as time went by, those rules and the side-effect thing became “self-evident” in the Scala community, mostly because that’s how all this was intended in the language from the inception. In other cases, like with the deprecation of postfix calls (before SIP-18), we referred to Martin’s tweet as a justification. Because there’s no appropriate Martin’s tweet (yet?) for what we’re introducing this time, we dare validate the step by ourselves.

We’re expanding the notion of “code style” for Scala beyond the typical constituents like spaces, parentheses and braces. For a start, we have introduced a special set of configurable rules of when to add / enforce type annotations (check Settings / Editor / Code style / Scala / Type Annotations). Truth be told, we added those settings about 4 years ago, but only now did we pick up our courage to enable some of those by default (on the other hand, you can’t say we’re rushing it :).

From now on, the type annotation settings will be used in the following functionality:

  • Introduce Variable / Field, Extract Method, Override / Implement Method refactorings,
  • Create Variable / Method from usage,
  • Java-to-Scala code conversion,
  • Reformat Code action with “Add type annotations” enabled,
  • Type Annotation inspection.

In particular, we’re considering enabling the enforcement of type annotations on public methods, and we’re definitely expecting a mixed reaction to that. Why declare some result types explicitly? Well, we believe that it’s one of those places where tools can mend possible abuse of certain liberties in the language design. Here are the reasons:

  • Public methods and properties represent a so-called “public API,” which is a form of abstraction. Forcing API users to investigate the inner workings of public methods (or to guess the result type) is a sure way to break encapsulation and to degrade code quality. Explicit type annotation make public API well-marked and easy to use.
  • Explicit result types can greatly improve code editing performance. Unlike the compiler, an IDE has to re-infer types on each code update. To optimize the workload, IntelliJ IDEA tries its best to cache and preserve as much type information as possible. However, as the result type of a method with omitted type annotation can change even on “external” update (i.e. update outside of the method body), IntelliJ has to re-process the whole content of such a method on almost any change (and the content might often consists of implicits, type-level arithmetic and whatnot). Considering the first point, adding type annotations to public methods makes your code more IDE-friendly (for the same reason, you may also choose to add explicit types to really complex yet non-public methods).
  • Type annotations on public methods increase the speed of incremental compilation by minimizing the amount of recompiled code. Algorithms of incremental compilers (like SBT or IntelliJ) track “public API changes” and dependencies to determine what files need to be recompiled. Thus, if type of some method in a frequently used class is adjusted after code editing (but still lays within some “implied” base type), almost the whole project might often be unnecessary recompiled as a result. In a sense, this argument is a combination of the two previous ones but from the compiler’s standpoint. Official SBT documentation supports this argument by saying that “explicitly annotating return types of a public API is a good practice in general.”

Note: One possible exception to these rules are simple declarations like val answer = 42 where the result type is clearly evident, both for humans and for the compiler. Currently we’re determined to treat this kind of expressions separately.

So, adding explicit types to public methods can make our code cleaner and also speed up editing / compilation. Sounds good, doesn’t it? But what about tons of those “righteous” warnings, aren’t they supposed to be annoying? The good news is that there is a straightforward way to process them all at once:

  1. Go to Settings / Editor / Code Style / Scala / Type Annotations and make sure that Add & Check option is selected for public members.
  2. Invoke Analyze / Inspect Code, select Whole project as a scope and create an inspection profile with only the Type annotation required inspection enabled.
  3. Run the inspection and then choose Apply fix “Add type annotation” to batch-add all the explicit type results.

When SCL-10501 will be implemented, another way to add required type annotations in batch is to invoke Reformat Code with Add type annotations enabled (but this also reformats code for good measure).

Because the functionality is already present in all the Scala plugin builds, you don’t have to wait for it to be enabled by default: you can tweak the code style and try this out right now. Likewise, if you consider the undertaking to be too much of a hassle, just tweak the code style settings as you see fit.

At the next step, we will add many more options to the code style (currently listed as inspections), so that it becomes possible to choose, for example, what form of anonymous function definitions you prefer (person => person.name or _.name) or to require eta-expansions to be explicit (like f _), and so on.

We encourage you to try the feature and, as always, we’d love to hear your feedback!

UPDATE 1

Scala Code Style now tells:

All public methods should have explicit type annotations. Type inference may break encapsulation in these cases, because it depends on internal method and class details. Without an explicit type, a change to the internals of a method or val could alter the public API of the class without warning, potentially breaking client code. Explicit type annotations can also help to improve compile times.

UPDATE 2

We’ve added fine-grained settings:Type Annotation Settings

Tooltip for the “Type is obvous” checkbox:Type is obvious

About Pavel Fatin

Programming enthusiast, technology advocate. IntelliJ Scala plugin developer at JetBrains, https://pavelfatin.com
This entry was posted in New Features, News and Events and tagged , , . Bookmark the permalink.

22 Responses to Beyond Code Style

  1. Leszek says:

    I think those are the right additions. You are right that you also create de-facto standards over time.

    Thanks for your work!

  2. Mateusz says:

    Interesting changes!

    Would it be possible to add support for scalastyle and scalariform? Or make it possible to easily import their settings into IntelliJ IDEA? This way code wouldn’t be formatted and checked twice in two different (possibly incompatible) ways.

    Thanks!

  3. SergeyB says:

    Well, my concern isn’t unnecessary simple types, but, instead, complex types that will pollute codebase. Example: any Slick query result or query builder’s intermediate type. It may have 5-7 nested type parameters and be 100 characters long.

    • Pavel Fatin says:

      Good point, thanks! As always we’re expecting some corner cases. At the first glance, we can employ one or more of the following:

      * Use a “sane” base type (we’re already doing that in Add / Remove / Update type annotation intention).
      * Introduce some type length threshold.
      * Hard-code useful heuristics for commonly used types.

      If there’s no sane base type, it could be a deficiency of particular library – Scala is not a dynamically-typed language – type inference is meant to omit obvious types, not to obscure inconceivable ones.

      The longer the type, the greater the benefit of specifying it (or base type) explicitly, so that IDEs don’t have to re-infer that complex type on every keystorke. Besides, the proposal is to specify only “public” types, while, for example, most Slick’s types are usually reside inside a method.

      Let’s wait for real-world use cases and then decide how to proceed. We encourage everyone to submit relevant examples to YouTrack.

  4. Nick Jacobs says:

    I don’t find it all that useful. The compiler doesn’t require it and I think the basis for that is that it’s noise most of the time for a human. If I really need to know the type in IntelliJ, I can always just click on it and press P.

    One of the things that I really like about Scala is it’s ability to infer types — I think this is a great example of the computer doing work for me as I want to see less code on the page, not more.

    • Nick Jacobs says:

      That should be Control-Shift P, btw. I guess the page strips < and > char’s. :)

    • Pavel Fatin says:

      “Because we can”, doesn’t necessarily mean that we should (for example, the compiler doesn’t require () for invoking def foo(), yet it isn’t a valid pretext to omit those). We’ve listed many sound technical reasons why we may prefer to state type of public entities explicitly, but there are many benefits for humans as well.

      To write clean code, programmers devise proper names, define temporary variables, extract complex code in separate methods, define abstractions, etc., whereas compiler doesn’t require any of that at all. While we can place the whole program in a single method and name things a and aa, that kind of “simplification” doesn’t make program any simpler for humans (on the contrary, that obfuscates code to the point of being completely incomprehensible). Most “noise” is needed exactly for humans.

      Explicitly defined public API is a way to achieve encapsulation, which is required for building robust abstractions. When method type is “implicit”, humans need to study the method code to understand what it actually returns – that breaks encapsulation for humans. Besides, if methods instantiates, for example, HashSet and then returns it, this HashSet will be propagated to all places where the method is used (and to the compiled code) – that breaks encapsulation for the compiler (holds true even for overridden methods accessed via non-base class). Explicit type defines a clean boundary that prevents that “leakage” of implementation details.

      As for the “Show Type” action (and other “Show …” actions): any need to use those helpers probably signifies that such a code lacks inherent clarity. One thing to consider here is whether the entity should be marked private in the first place, in which case it might be acceptable for the code to be less clear. We’re currently working on a corresponding inspection to detect such cases automatically.

      That said, one can always apply any style freely, as it’s possible to fine-tune the style, or even to disable all the checks with a few clicks. While we’re trying to do a reasonable thing in general, realistically we understand that no setting can fit 100% of the users and customizability is essential.

  5. Stefan Schulz says:

    This new feature is quite noisy, and frankly it robs the code of a lot of its elegance. I would rather have it disabled by default. While the benefits to the IDE might be worth it, complex expressions from slick and/or scalaz quickly become much longer than the “actual” logic. I for one disabled the warnings, and am not likely to turn them back on.

    Having inferred types is especially valuable during the initial development of a method. Frequently, I am not initially quite sure how to size the method, and the type may change quite frequently. With an inferred type, as long as caller and callee are compatible, things work out fine. If I need to update a method return type and a variable type as well, 1-2 updates become 4.

    IMHO you are robbing the language of one of its key features. Even java moved tentatively towards a little bit of type inferrence, and you are pushing scala away from it? I would ask you to reconsider.

    • Pavel Fatin says:

      Hi Stefan,

      Could you please post the concrete examples to YouTrack. Aren’t those methods should be private in the first place? Can we add a heuristics that suggests concise base types? If there’s no sane base type, it could be a deficiency of particular library (as Scala is not a dynamically-typed language – type inference is meant to omit obvious types, not to obscure inconceivable ones). Or, maybe we could add (configurable) exceptions for particular types…

      We surely agree that you have a point, but then again, we never expected that “one size fits all”. Some projects tend to stretch Scala’s flexibility and the default inspection settings might not blend with them too well – in such cases we definitely recommend to disable the highlighting (one can even modify the default inspection profile). However, those projects seem to be in minority, and we believe that the current scheme is a middle ground.

      As for the “type inference trend”: Java considers only local-variable type inference, not inference of method return types. In the same way, Kotlin (which had a luxury of hindsight) states that “Kotlin does not infer return types for functions with block bodies because such functions may have complex control flow in the body, and the return type will be non-obvious to the reader (and sometimes even for the compiler).” I.e. those approaches are much more restrictive.

      By default, we only recommend annotations for public members and only for non-obvious ones (because public members without explicit types are problematic on so many levels). It’s not only about IDEs – we really think that it’s a reasonable thing to do and majority of projects will benefit from it.

      As to IDEs – some Scala features were designed without an IDE in mind and if a project (e.g. Shapeless) takes ages to compile, how on earth an IDE is supposed to analyze it in real-time? Adding at least some type annotations helps to make project more IDE-friendly and it’s a very practical thing to do if you want to gain from on-the-fly code analysis.

      • Stefan Schulz says:

        Unfortunately most of my actual examples are not code I can publically talk about. Yet much code we have looks similar to this: http://pastebin.com/Y4mZQcK1

        You probably agree that the type annotation is not helping anyone to understand the method. This gets worse once you consider more complex queries including joins and mappings.

        I agree with you that a lot of the more complex examples really stress the type systems in ways the designers probably did not quite anticipate. I also agree that I cannot expect real-time responses from an IDE for code the compiler needs to “think about” for five minutes. Maybe an acceptable tradeoff might be to exclude some libraries or frameworks which often feature very complex types from this inspection? It might help define cleaner interfaces where they are sanely possible, and otherwise skip them.

        Maybe track how long the internal engine needs to figure out the actual type, and skip the inspection once a threshold is exceeded?

        • Pavel Fatin says:

          Hi Stefan,

          Thanks for the example (though, it’s hard to understand all the details without the real context) and the suggestions!

          That inconceivable type might be a specific (or even a limitation) of the particular library. By the way, could the getTransactionExplicit value be made private?

          We’re currently working on a UI that will allow to configure the highlighting rules more flexibly (particularly, to handle particular libraries and frameworks), so, I hope, we should be able to get the best of both worlds.

    • Jeffrey Aguilera says:

      Yes, I agree. It is much too noisy. Over time, I have slowly but systematically disabled all the “Type Annotation required rules” due to false positives and the low value provided by that inspection. For instance, I would expect implicit val timeout = Timeout(5.seconds) to have an obvious type of Timeout. The “Except when [ ] Type is obvious” setting behaves otherwise.

      • Pavel Fatin says:

        Hi Jeffrey,

        The inspection does infer “obvious type” from Timeout(5.seconds). However, any implicit definition is required to specify type explicitly (by default). Dotty compiler, for example, requires type annotations on implicit definitions and would stop with an error “resulttype of implicit definition needs to be given explicitly” even when right-hand side expression is as simple as 123.

        BTW, we’ve added more detailed inspection messages that explain why a type annotation is required in each particular case (this feature will be included in the upcoming 2017.2.* release build soon).

  6. Suiram says:

    There should be a possibility to restrict this, e.g. only in traits, don’t require annotating the apply method, don’t require it in nested or private classes, and so on. Otherwise it’s too noisy and I don’t like the green error annotation all over my code base.

  7. Pavel Fatin says:

    Oh, that’s nice — now Scala Code Style tells:

    All public methods should have explicit type annotations. Type inference may break encapsulation in these cases, because it depends on internal method and class details. Without an explicit type, a change to the internals of a method or val could alter the public API of the class without warning, potentially breaking client code. Explicit type annotations can also help to improve compile times.

    • Michael says:

      Sadly, new settings don’t allow omitting type annotations when a public member is overriden.

      • Pavel Fatin says:

        Hi Michael,

        The initial version of the settings was created many years ago, when we assumed that members that override something should be treated in some special way, thus we added those additional settings. However, now it seems that such members should be handled uniformly, because Scala ascribes actual type, not base type during type inference, for example:

        Such a code breaks encapsulation (for example, prevents Bar to be declared private) and might trigger unnecessary recompilation.

        We considered allowing to omit type annotation when right-hand side has the same type as the base declaration, but in such a case further refactoring could break that correlation without anyone noticing, due to “action at a distance” (nobody regularly verifies each source file). Besides, adding more special cases can obscure the already tricky settings UI.

        It seems, that the only case where something like this is permissible is when the type is immediately obvious from the right-hand side expression and also “stable”, e.g. def foo = new Foo() (the “Type is obvious” checkbox is enabled by default).

  8. Maxim Buzdalov says:

    Hello guys,

    The latest logic change to force all implicit values, regardless public or private, being explicitly typed, is of course understandable, but it turns off some beautiful possibilities of scrapping the boilerplate in Scala.

    This is an example of a configuration of a genetic algorithm, in a sort of DSL I have developed for it some years ago:

    https://gist.github.com/mbuzdalov/5ec3a8fdee3ab7d3889c6af0e1a31787

    A typical genetic algorithm has lots of global singleton things, such as the type of genotype, the type of fitness value, the fitness function itself, and so on. It feels natural to be able to write them down as implicit values, which other components can just see.

    Note that all implicits with inferred types are private here, so nothing escapes the current scope.

    All these types have to include the types of genotype and fitness. If one has to specify them every time, this becomes painful:

    https://gist.github.com/mbuzdalov/b8517ccfc6cc9e403a2959eca5c403c3

    It is painful both to read and to maintain – if one just wants to alter the fitness type (e.g. change IndexedSeq[Int] to IndexedSeq[Long]), one changes nearly nothing in the former case, but has to fix every type in the latter case.

    For a while – if I am correct, before 2017.2 – everything was OK (public inferred implicits are warned about, private ones are left unnoticed). I would prefer if it continued to be this way.

    • Pavel Fatin says:

      Hi Maxim,

      An implicit definition without an explicit type is a separate concern regardless of whether such a definition is public or private.

      Dotty compiler, for example, requires type annotations on implicit definitions and would stop with an error “resulttype of implicit definition needs to be given explicitly” even if the definition is declared as private[this]. This is the major reason behind the specific settings “out of the box”.

      If you want to allow implicit definitions without explicit types, you can easily do that simply by clearing the “Implicit definition” checkbox in the settings (that’s why the settings exists, after all). In that way, it’s possible to hide the implicit-related warnings while still preserving the access-related ones (moreover, it’s possible to do that on per-project basis, as the type annotation settings are not “global”, but rather a part of Code Style, which could be applied at the level of particular project).

  9. Michel Charpentier says:

    An example of why it’s good to write return types explicitly on public methods:

    GroupIterator implements Iterator[Seq[B]] and many of its methods are in terms of Seq[B]. For instance, toList returns a List[Seq[B]]. The exception is next, which returns a List[B] instead of a Seq[B].

    The code below works, but really shouldn’t. And I don’t know if a future type annotation on next might break it.

    scala
    val i: Iterator[Int] = ...

    i grouped 2 withPartial false map {
    case x :: y :: Nil => x + y
    }

Leave a Reply

Your email address will not be published. Required fields are marked *