I have this (simplified) code example:
File.write("out.txt", "content", mode="a")
which is not what I intended. It should should have been using a colon as in this one:
File.write("out.txt", "content", mode:"a")
However the former still works and creates a file with the following permissions:
---xr-Sr--
I wonder if such code snippet could be flagged as incorrect or if there is any tool (a linter maybe?) that will flag such code as “probably incorrect”.
Yes, you write a test that proves the code does what you want it to do.
There’s also a linter called ameba that might catch these things, but I haven’t used it.
ameba is excellent. It catches this type of error with the Lint/UselessAssign rule. Thank you for the suggestion!
Great! And sorry for that test suggestion, it has nothing to do with what you asked 
Maybe it’s finally time to make that a warning in the compiler itself, and requiring parentheses around that if really wanted.
For me the discussion would be whether we want to have a linter as part of the compiler or not. I don’t think discussing this for every particular linting feature again and again is very worthwhile :)
Well, in other languages I’d write a test for the “null reference checks” as well, but when you are spoiled with the compiler doing the hard work for you, you demand it all 
If I, as a total newbie to Crystal, could chip in: It all depends on the cost. If the linter makes the compiler too slow (whatever that might mean) then it is better to leave it out. Let the developers configure the linter in their IDE, a pre-commit hook, or in the CI system.
Personally I just added it to my CI system.
It’s not so much about (performance) cost but overall complexity. Implementation wouldn’t be hard to do, but linting is always about style and that’s highly subjective. The compiler’s formatter tool for example represents very basic styling rules that can be expected to be followed by the vast majority of Crystal projects. But everything beyond that is really hard to do because finding a consensus is almost impossible. That’s why linting is better suited as a stand-alone tool, allowing a good amount of flexibility to adjust to one’s liking.
So the main action point here is IMO to just recommend using ameba (or any linter, in case there are others).
Regarding the compiler, the only option that sounds somewhat reasonable to me is making assignments in method call arguments invalid (as mentioned in Named arguments usage ambiguity). Considering this is a common pitfall (I remember falling into as well, a long time ago) and there’s probably not a really striking reason to allow this, it might be better to just make it an error.
Let me show two other cases that are “most likely bugs”
The first is a conditional in void context (x == 22). Neither the compiler nor ameba catches it.
The second one is assignment in conditional. Amebe reports it as “Useless assignment to variable x” which is actually an incorrect categorization IMHO as the assignment works, but that should be probably a comparison. I’ll report these issues to Amebe as well.
x = 23
x == 22
if x = 42
puts "OK"
puts x
end
Actually, that second case is intended behavior. It’s not obviously useful in the code you give, but consider this case:
arr = [1, 2, 3, 4, 5]
possible_three_index = arr.index(3) # => could return nil
if three_index = possible_three_index # => constrains three_index to Int32 from (Int32 | Nil)
puts three_index
end
Now, this isn’t necessarily a great example in practice (you could just use #not_nil!), but the situation it demonstrates actually comes up quite a lot in Crystal, especially with nilable instance variables.
However, I suppose a linter might be able to flag probable errors of this type that are meant to be == if the linter can figure out that the variable isn’t nilable.
And the first case is probably lintable.
comparisson expressions alone like x == 22 are useful on return values like:
def is_22?(value) : Bool
x == 22
end
But I guess this could be an ameba warning when the expression is not used in return values.
And since I’m writing here… IMO the linter should be a separate project for the reasons people said above as it is now.
I agree, this should NOT be a ameba warning, but if and when the function is used in void context that should be. So in case there is some code like this:
is_22?(23)
IMHO.