Pre-SIP: warn against unidiomatic features (e.g., null value and unsafe casts)

I think the biggest culprit here is really IntelliJ, which inserts language imports automatically. That completely nullifies any educational value such imports have.

Has that been fixed yet? If not, I would suggest to raise the issue with the IntelliJ Scala plugin team.

2 Likes

So by this logic, that people misuse their compiler to use null or asInstanceOf or whatever, should not be reason enough to make the rest of us forego (easy) access to it!

Right?


Personally, I find language imports a constant irritation. I have to remember over and over and over to set the right imports in order to spare new users the one-time cost of learning how to use these things responsibly.

Having even more is unwelcome. (If we were to have another one, I would agree with Rob that making case o: Option[Int] an error instead of merely a warning would be my first choice.)

It’s painful enough to have to deal with null from Java without having the compiler yell at me. I already know; I don’t want to use it. I can’t always avoid it.

It’s already too painful to type all of asInstanceOf. Really, I’d rather match; if I’m doing that, it’s for good reason. Making even worse is gratuitous.

@unchecked and @uncheckedVariance again are things you only run into when you really need them. You can’t stop a certain fraction of users cutting and pasting random stuff around until the code compiles, but language imports aren’t the place to solve this problem: mentorship and code reviews are, possibly in conjunction with a linter.

Anyway, as is evident from what I’ve said already, I am not in favor. I like the language imports feature to warn about aspects of the language that you can accidentally stumble into and be badly burned by (e.g. postfix ops, reflectively-accessed structural types). These aren’t those cases, though.

Scala isn’t a completely general-purpose language, but making it more awkward to use in more scenarios is not, I think, a winning strategy. I’m all for teaching good practice, but making everyone who already knows what to do / not to do suffer forever as a consequence should be a measure of last resort.

1 Like

I’m just going to chip in and say that language imports are a terrible idea, I have not ever seen a single codebase which language imports have made easier to navigate, have not ever seen a single file whose language import helped in any way understand.

There are things that should be warnings, and there are things which should require a programmer to explain why they are doing something unusual, there are things which you’d want to leave extra documentation about your intention, and there are things you want to discourage. There is work you can do around squelch-able warnings that can serve all of these purposes. Language imports serve none of these purposes and should be removed.

5 Likes

It is kind of ironic that in the same thread where we discuss increasing guidance for coders, we also lament IntelliJ’s intervention. If you like more guidance, you should love IntelliJ!

IntelliJ guides coders all the time, flagging suspicious code and suggesting improvements.

For example, given the following code:

** def sillyToString(x: AnyRef): String = {
if(x.isInstanceOf[String]) {
x.asInstanceOf[String]
} else {
x.toString
}
}**

IntelliJ suggests to replace this by a pattern match, and if I accept the suggestion, turns it into:

** def sillyToString(x: AnyRef): String = {
x match {
case str: String =>
str
case _ =>
x.toString
}
}**

Note that not only the isInstanceOf, but also the asInstanceOf is gone.

If IntelliJ does not flag every instance of null or asInstanceOf, then that is probably because (1) the code probably does exactly what the coder intended it to do and (2) there is no obvious alternative. And in that case, a compiler warning probably wouldn’t do much good either, because the coder either does not know the alternative, or decided the alternative was not good enough.

Too many compiler warning are not good, because it increases the chance that they will be disabled or ignored, especially if coders start to feel that the warnings are probably not bugs, but just perceived lack of idiomaticity.

Compiler warnings should be reserved for cases that are probably bugs, because only then people will feel compelled to take them seriously.

Guidance for code improvements should go to tools. Perhaps the compiler may emit, next to errors and warnings, also “advises”.

1 Like

Language levels have existed in Racket and its predecessors for at least a decade and have been very successful at guiding people through increasingly expressive versions of Scheme. So it definitely works if you do it right.

I think the error we made was hiding features that are likely to be unfamiliar rather than [also] features that require expertise to use safely. Low-level constructs like null and asInstanceOf and less-common things like var and while, and truly weird things like return are familiar to Java programmers but they’re advanced by any reasonable definition: they make programs harder to reason about and make it easier to write bugs, and are mostly for specialized cases for expert users who know what they’re doing. I don’t use any of this stuff because it’s too hard, and I have been writing Scala for a long time.

Anyway I’m a fan of language imports in principle. Scala is a huge language and people need help staying in the safe zone.

1 Like

I don’t think any of us, especially those who’s been using Scala since before SIP-18 was introduced in 2012 find the language imports to be particularly useful. But take a look at the language object today:

  object language {
    import languageFeature._
    implicit val macros: macros = _
    implicit val dynamics: dynamics = _ 
    implicit val postfixOps: postfixOps = _ 
    implicit val reflectiveCalls: reflectiveCalls = _
    implicit val implicitConversions: implicitConversions = _
    implicit val higherKinds: higherKinds = _
    implicit val existentials: existentials = _
    object experimental {
      implicit val macros: macros = _
    }  
  }

Compared to 2012 ~ 2013 when many people were using all the tricks to extend the boundary of programming, I feel like the standing of these language features like postfix operations, implicit conversions, and macros have lowered in the last six years. Some of these are not even making it to Scala 3. So, in my opinion, it has done it’s job as well as it could have. There might also be a network effect of signaling to educators and book authors that they are not recommended.

Then again, I think we should make linting tools like Scalafix better that can tailor to the specific styles and policies of your team or project, instead of adding more to the Scala language spec in this direction.

I agree with you in general, but addition of specific styling to the language specification is a thing that is default for those who are not using tools like Scalafix.

Maybe, compilation and styling (including idiomacity warnings and language features turning on/off) should be fully separated (to make each tool to do only its own thing), but it means that a styling tool should also come with compiler and should be integrated well (for instance, to set the used style in build.sbt).

And then, if you haven’t set any style settings (e.g. in your build.sbt), then some default idiomacy styling would be applied. But this would be done not by a compiler but by a special tool which is more flexible and tunable than a compiler.

I’m not sure where this idea comes from. It’s certainly not how IntelliJ behaves in my experience. (Maybe there’s a setting to add it silently but if it exists it must need to be enabled by the user – I have no recollection of turning off any such option.)

In my experience IntelliJ acts as follows: When you use features that require an import, it highlights it as an Inspection:

Then you can click the lightbulb or press Alt-Enter which gives you this popup:

image

Enable implicit conversions seems to just make the highlight go away, but doesn’t seem to persist – after closing the project and reopening it the highlight was back.

The second option will indeed add the import statement for you. However I would hardly call it automatic on the whole – you have to perform some keystrokes or mouse clicks in exchange for not having to type the exact import line.

BTW clicking more in the first tooltip, or pressing Ctrl-F1, shows this:

In conclusion, to me they’re doing a decent job of preserving the spirit and message of SIP-18 – these features shouldn’t be used unless you know what you’re doing, in which case you should say so explicitly.

2 Likes

That’s very sensible behavior. What I saw before (maybe 2 years ago?) did not do this. I seem to remember it just added the import. But I might be wrong. Anyway I am glad it’s the way you describe it now.

1 Like

For some reason I thought this was about errors, but it’s just about warnings, so I retract my previous statement.

These pointless language imports most certainly annoy me. And while you can put the compiler options into the sbt and mill build files, when other people open the project in an ide they may get the warnings. These warnings also seem to appear in mill and sbt’s out put from compiling themselves. I don’t think its such a problem with experienced Scala programmers, but when you are introducing new people to the language, you don’t want these distracting warning messages.