Crystal: More helpful error message when trying to define a constant in a method

[meew0]

Currently, the error message when defining a constant inside a method is Syntax error in <location>: dynamic constant assignment. This is a pretty unhelpful error message, as it says nothing about the actual issue, which is that you can't define a constant inside a method. It would be good if this error message were improved.

Simple code sample that causes the error, if necessary:

def foo
  BAR = 1
end

puts foo

(https://carc.in/#/r/2j8a)

[straight-shoota]

To me the error message sounds pretty descriptive of the error: You can't define a constant in a dynamic context like a method body. It could probably elaborate more on that.

[ghost]

To me the error message sounds pretty descriptive of the error

This is not good because it is a matter of personal opinion. You may find x to be sufficient,
another user may not find it sufficient.

I do not know the situation on crystal since I use ruby primarily but one pet peeve of mine
in ruby is in general the whole warning system, which I find not really that great.

(I do not know if you can avoid the above warning in crystal... does .set_const() work
to still set the constant value?)

One suggestion I have made to the ruby core team was to be able to more easily
fine-tune the whole error message system. A customizable system would help in the
sense that users could decide on their own how verbose and "helpful" they want to have
errors occur. In this context, meew0 could then set a "verbose errors" somewhere
and that would probably solve his case (a global setting for example; and also a
way to globally revert to the deault).

I do not know how the crystal team views the whole issue in regards to errors but
this is something where I think in general, improvements may be helpful to users.

I agree with straight-shoota in one regards by the way - meew0 has not said
HOW exactly the message should be different. :)

[meew0]

@straight-shoota This is the first time I've heard of the term "dynamic context" like that; perhaps the message is descriptive for someone who knows that term but I imagine most people wouldn't.

@shevegen Maybe "constants cannot be defined in a method body"? Or even, if it's important to have "dynamic context", "constants cannot be defined in a dynamic context, like a method body".

[straight-shoota]

Well a dynamic context is the opposite of a static context. And it is quite logical that constants can only be defined in a static context i.e. at top level or inside type (class scope), see docs. A dynamic context like a method body would not be executed right away (or never at all) and therefore cannot define a constant (because it wouldn't be a constant then).
Constants cannot be defined in method bodys, blocks, begin/end-blocks, conditional branches, loops, conditional expressions etc. It makes no sense to name all of them and Im not sure it's worth customizing the error message to include a description of the current context type where it is not allowed to define a constant. But that might be an idea, if someone is interested...

And I strongly disagree that error messages should be in any way customizable. Ignoring the extra effort this would require in implementation and runtime complexity, it's just a really bad idea to have different messages. It makes it hard to find error reports about the same error but with different wording.
The error messages should be concise and complete by default without any need to provide alternatives.

[ysbaddaden]

The message is to the point: "errror: dynamic constant assignment" but may be cryptic to the developer, especially for newcomers that are used to languages with relaxed "constants", such as Ruby.

Maybe the message could be evolved to something more helpful to the developer, for example:

SyntaxError: a constant can't be initialized in a dynamic context (e.g. inside method definition, conditional if branches, ...).

[straight-shoota]

I'd rather point to the places where you can define constants instead of giving examples where you cannot. We might just take the wording from the language reference:

SyntaxError: dynamic constant assignment. Constants can only be declared at the top level or inside other types.

[vegai]

Generally speaking, I don't think there's such a thing as talking too much in error messages -- as long as every sentence brings in new information. Seems to work pretty well in languages like Elm, where compiler errors are quite verbose and people celebrate them

[crisward]

Looks like this just needs updating here and here and the tests updating. I don't mind doing a pull request with @straight-shoota suggestion if it helps.

[asterite]

@crisward This suggestion ( https://github.com/crystal-lang/crystal/issues/4831#issuecomment-322787206 ) looks good, so go ahead if you want to. Thank you! 🙇