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

I think If I try to explain why it can be comfortable I will recive many arguments like

  • you are wrong it is not scala way plenty of tools in Scala to interact with Java.

At the and I will recive somethink like

  • Let’s get back down to earth

Sorry I am not ready. I think who want to understand my position already can do it.

You’re wrong about that. I for one am trying to understand your position, but can’t see it.

May be, but I do not know what to say else.
I can only repeat
We use null when

  • use java library
  • performance (also in everyday business logic)

am not ready to argue that I can write something like

val o = option(file.getParent)
 o match {
    case Some(p) =>  
       ...
}
//instead of 
 val p= file.getParent()
 if (p!=null){
 .... 
 }

I think warring is bad because:

  • there are no better alternative in such cases
  • It add much more complexity for library user than for library authors. Authors of library can just ignore such warning. The main additional works will fall on users.

I do not like a suggestion which does not suggest better alternative, instead it suggests more difficulties.

At the end people can just disagree :slight_smile:

Sure, you can do

Option(file.getParent()) match {
    case Some(file) => ....
    case None => ....
}

That way you handle all cases properly

The point of the SIP is not that you should never use the feature because there is no better way to do it (whether there is in your example depends on the rest of the code, maybe there is, maybe there isn’t, maybe it’s a matter of opinion), the point is that you get a warning if you don’t explicitly enable the feature with either an import or a compiler option.

I don’t believe adding a language import adds much complexity for the library user at all. To me, it seems entirely straightforward, especially since the warning points towards the language import.

You import the language feature to tell scala - hey, I’m about to do something unidiomatic with the language, and I’m going to do so anyway because I need to interop with Java.

You add the compiler option because you do it all the time.

The same as it’s now with existentials, higher kinded types (though I believe that’s a mistake), dynamic objects and reflective calls.

They’re part of the language, there are cases where they are the right choice, and you should totally use them in those cases. But there are many cases where they are not, and you should be warned that you’re doing something mildly funky, unless you turn off the warning by import or compiler flag.

2 Likes

That’s a good point. I am pretty sure I tried it, but under some conditions I still managed to break it - not sure if I got null or I wasn’t able to order actions as I needed (sometimes construction was branched and I don’t think one can check lazy val if it is already initialized without triggering initialization). Right side of val was calling a method which constructed a widget and added that widget to the current widget (to this, order of adding is important). Maybe converting everything to lazy would have solved my issue, I am not sure. The problem is that it is not that easy to reason about and debug code where everything is lazy compared to explicitly calling init/create methods in traits from a base class. Plus, as you mentioned, the performance cost could be too high.

My point was that while = _ is not considered idiomatic, it can still be used heavily in some codebases and since Scala tries hard to be compatible with Java world we should not presume only beginners use null or = _ (in my case I was using a Java library, but I think there are also types of projects which cannot afford to use other approaches because of performance cost even if they are in pure Scala).

Honestly, I would prefer having both. And for the project I was mentioning, it would be incredibly useful having also something package scoped (e.g. disable it for all UI stuff, but keep it enabled in game logic), but I don’t know if many people would use it.

Does importing the language import in the package object work for that? I don’t know if it does, or how I feel about whether it should. It’s a separate discussion I suppose, but good to take along.

TBH, I think this entire pre-SIP is moot. Not arguing about the idiomaticity of such constructs, or whether they’re sometimes useful, or whatever. But we already have experimental evidence that language imports have solved nothing. They are not the deterrent their authors intended them to be. People just import them, or add codebase-wide compiler options, and never stop to ask themselves “hey, maybe if I have to import something, it’s because that feature might not be what I’m looking for”.

Putting more stuff under language imports will accomplish nothing.

4 Likes

I’m not aware of this evidence, and anecdotally, they have been useful to me, particularly around existentials where I now more often use a type member instead.

What evidence is this?

1 Like

I try to describe.

  • Write code,
  • make commit,
  • on every worning make import(additional work and nothing more)

I agree it.
There is nothing except more long commit phase.

To be more clear, people have benchmarked this. There is a big performance hit when the lazy val gets evaluated on the first hit (because it does it its best to try and do it properly i.e. checking for circular doublelocks as well as other threads) however for any subsequent reads of the lazy val the performance impact is almost negligible.

Agreed, it is the classic initialization issue (although some people would argue that using Option may be better)

1 Like

I find this feature useful (if it would be customizable enough - at least per file and project). It could not only nudge beginners (not all, but experts coming from other language would IMO read about the warning instead of blindly suppressing it), but also warn advanced users who made a mistake (e.g. null in a file where it should not be, because it doesn’t interact with Java code). I am not sure it has to be in the compiler itself (but that is from my perspective of no longer being a beginner), maybe a separate lint tool would be enough.

I think Martin at one point mentioned this, basically with people using editors like Intellij (which have an inspection that just adds the import if you are using an advanced language feature) it really hasn’t deterred people from using such features. They just add in the import and use the feature.

This is a general language design issue, and some languages (such as Go) have decided to make certain unidiomatic things errors. i.e. unused imports/variables are actually compile errors in Go, you cant actually try and hide/ignore them with some import.

I’d still be interested in seeing this evidence. If anyone knows where to find it, I’d love to see it.

My IDE can do all sorts of stupid things if I let it. The fact that some users misuse their tools and refuse to think about what they’re doing should not be reason enough to forgo a feature that can prove useful to the rest of the community, IMHO.

2 Likes

I’d also like to see the experimental evidence.

You can’t just make this kind of sweeping statements without substantiating them. Otherwise, they just sound borderline condescending or even brash.

This is anecdotal evidence but I, for one, would have probably not known that so-called reflective calls are terrible and use reflection, if the compiler had never pointed it to me. I don’t think it’s unreasonable to imagine that this is the case for many other people, and so there is probably at least one instance where language imports were useful.

4 Likes

OK, sorry, I spoke too strongly. In particular, I reviewed the definition of “empirical evidence”, and clearly using that word was not justified. I retract that statement.

It’s great if some people find the language imports useful. In my experience, though, virtually all the feedback I have seen over the years is that it’s at most visual annoyance at the top of the file, assuming your IDE does not hide them.

The reflective calls example is perhaps special, because the warnings point to code which has unexpectedly poor performance. It looks like a method call, but’s terrible performance-wise. It’s especially relevant since some calls to refined methods can still be compiled as efficient method calls (for example, the apply method of dependent function types in Scala 3), so sometimes it’s perfectly fine to use them, but you want the compiler against unexpectedly slow ones.

Similarly, warnings for existential types that cannot be expressed with wildcards are good, because they don’t exist in Scala 3 anymore.

However, null is not going to disappear, nor implicit conversions, nor asInstanceOf, nor do any of them have unexpectedly poor performance. Therefore, the warnings are much more likely to be only annoyance, and therefore disregarded.

2 Likes

We have two departments. One for high level business logic other for automation the work of the first one.
And I have seen many times such discussions very closely.

The most hot discussion is always when one man really believe that he know what others people really need it. Of course he more qualified and he better know. And of course others do not understand what is good and bad, what is important and what is not important.

I think everybody can think everything :slight_smile:
But when someone try to prove his position with such words, I think this is the favorite words of white knight( A person who need to aid others who may or may not need it). So may be you are right. But I always want to say please do not bother about others. Because I have seen many times how the road to hell is paved with good intentions

Of course It is not about null. It is too small task for that :wink:

It’s a chess piece.