Crystal: Annotation[] should accept StringLiteral

[vladfaust]

We have def [](index : NumberLiteral) and def [](name : SymbolLiteral), but a StringLiteral should be allowed as well as we're going to get rid of symbols in the future (I'm already using strings as NamedTuple keys everywhere).

[MakeNowJust]

Following NamedTupleLiteral#[] behavior, Annotation#[] should accept every ASTNode and should handle StringLiteral and MacroId. IMHO it is more consistent than extending Annotation#[] to accept StringLiteral simply.

[vladfaust]

Don't know about NamedTupleLiteral, but accepting ASTNode doesn't make sense for me. It's cleary should be a compilation error.

[MakeNowJust]

I don't know why NamedTupleLiteral#[] accepts ASTNode. And it doesn't make a sense really. One reason I guess is backward compatibility for HashLiteral.

@asterite Could you remember about this?

[asterite]

I probably copied it from HashLiteral.

As for Annotation, maybe it should also accept MacroId and StringLiteral. But I wouldn't worry too much about it, annotations are experimental and I (personally) wouldn't add anything more related to them.

[vladfaust]

Are there any plans on removing annotations? I'm starting to love them, they bring many opportunities and I'm already rewriting an ORM and https://github.com/vladfaust/params.cr to annotations

[asterite]

@vladfaust No plans yet. Just that because they weren't thoroughly planned, I personally (and I say this from my point of view) wouldn't touch them until a good design is cooked.

[vladfaust]

@asterite but the whole idea of storing data on objects in compilation time would remain the same, right?

[asterite]

@vladfaust I don't know. I'm currently not thinking about the design of Crystal, I just know that we shouldn't continue changing stuff without thinking them well.