Allow multiple annotations in the same annotation block

[beta-ziliani]

Currently, each annotation takes its own line, leading to code that is not as comfortable to read. Made up example:

@[Nullable]
@[Unique]
@[Default(0)]
@[Type(smallint)]
property MyKey

@[NonNullable]
@[Default("")]
@[Type(string)]
property Data

Consider instead the following version:

@[Nullable, Unique, Default(0), Type(smallint)]
property MyKey

@[NonNullable, Default(""), Type(string)]
property Data

To me, I can easily read that there are two properties, and I can check the details just if I need to. In the first version, I have to go through more lines to get the most important information.

[naqvis]

I would say this would cause confusion as current way of annotating is how other languages are following the practice. Also this approach will cause clutter when combining multiple annotations which comes with elements.

instead it would be much cleaner if one can annotate an annotation and then use that annotation instead. Something like (made up example)

@[Nullable]
@[Unique]
annotation MultiAnnotation
end

@[MultiAnnotation]
property mykey

[straight-shoota]

This syntax should be doable, but I'm not convinced it's a good thing. I would strictly prefer each annotation on a single line. That makes more sense to me to clearly tell them apart. It's easier to miss an annotation if it's hidden behind others.

Maybe this can be combined to writing annotations in a list, but formatted with line breaks?

@[Nullable,
  Unique,
  Default(0)
  Type(smallint)]
property MyKey

@[
  NonNullable,
  Default(""),
  Type(string)
]
property Data

Not sure if that has a reasonable benefit over the original multi-line notation.

[HertzDevil]

We should allow multiple annotations on the same line:

@[Nullable] @[Unique] @[Default(0)] @[Type(smallint)]
property my_key

There is no need to expand the syntax of a single annotation node to allow this.

[Sija]

We should allow multiple annotations on the same line:

@[Nullable] @[Unique] @[Default(0)] @[Type(smallint)]
property my_key

That's IMO the least readable version of all proposed.

[beta-ziliani]

I don't know if it's the least readable, but I certainly believe the original proposal reads much better

[vlazar]

Putting aside the typical semantic of | and considering only visual representation I like how this looks in terms of readability:

@[Nullable | Unique | Default(0) | Type(smallint)]
property MyKey

@[NonNullable | Default("") | Type(string)]
property Data

Compared to

@[Nullable, Unique, Default(0), Type(smallint)]
property MyKey

@[NonNullable, Default(""), Type(string)]
property Data

Sure, you can write

@[Nullable , Unique , Default(0) , Type(smallint)]
property MyKey

@[NonNullable , Default("") , Type(string)]
property Data

But it less readable and different style with commas.

[n-pn]

how about using semi-colons as separator? Some people like me has been grouping related Enum options into a single line like this already, so our eyes are already trained for this..

@[Nullable; Unique; Default(0); Type(smallint)]

also, nothing can stop us from using multiple annotations when it's more appropriate, like this:

@[NonNullable; Default(0); Type(int)]
@[Unique; UniqueConstraintName("unique_index")]
property name

[spuun]

To me the brackets make it look like i could put more stuff in there, like in c# and PHP.
If it was only @MyAnnotation (like in java?) it wouldn't make me think that I could put multiple annotations in one block (because there's nothing that can group the annotations).

[beta-ziliani]

I'm failing to see the issue with the comma-separated list from these examples. I do agree it might be an issue with multiple arguments, but those aren't so common. And the notation [a, b, c] is consistent with a list.

[straight-shoota]

I'd like to learn a bit more about real use cases. The examples are just made up.

In my projects folder, I found only a hand full of more than two consecutive annotations. They are from https://github.com/ujjwalguptaofficial/shivneri and https://github.com/athena-framework/athena, both frameworks which heavily use annotations.

I'm showing one example of each. They have the max number of consecutive annotations, but the complexity and length of each single annotation is representative.

https://github.com/ujjwalguptaofficial/shivneri/blob/bafc1b03ca55b1acaaff97aa7e40d816c62a60b0/tests/general/src/controllers/user_controller.cr#L39-L44

@[Worker("POST")]
@[Route("/")]
@[Guards(UserValidator)]
@[Inject("as_body")]
@[ExpectBody(User)]

https://github.com/athena-framework/athena/blob/bfba452dd40bfd515c9358723fd7e8a64bbf7f8f/spec/controllers/routing_controller.cr#L88-L90

@[ATHA::Get("events")]
@[ATHA::ParamConverter("since", converter: ATH::TimeConverter)]
@[ATHA::QueryParam("since")]

These real annotations are noticeably longer than those in the made up examples and occupy a higher fraction of the line. Which reduces the space advantage and would affect the readability of collapsed notation in a single line.

In my opinion, none of these examples would improve by collapsing the annotations, regardless of which syntax we use.

[oprypin]

My main input here is that in the past years I've seen auto-formatters of all sorts usually converge on a "single thing per line" syntax.

In an alternate universe where Crystal already had both syntaxes, I'd expect some amount of complaints coming in from teams being unable to conclude on a consistent way to write these.

Even from the main post of this issue, the first example is actually the way that I'd prefer things converged on. And it has the huge advantage of being the only way to write it.

And this isn't even fixable by Crystal's formatter, the point of which is to have one undisputed way to write things.

[spuun]

This is an example were I wanted to put multiple annotations in one block.

I made a config class were i wanted to annotate some properties if they could be changed on reload, should be included in the diff between current and new config, and if it can be set via command line argument. I can also imagine that I'd like to add some validation related annotations at some point.

It looks something like this (but a lot more options):

class Config
   @[NoReload]
   @[NoDiff]
   @[CliOpt("-d dir", "--data-dir=dir", "Data directory")]
   property data_dir = ""

   @[NoDiff]
   property users = Hash(String, String) .new

   property log_level = Log::Severity::Info
end

IMO it would have been more readable if i could put the simple annotations in on block. Now it almost feels like the properties disappears because of the annotations.

[beta-ziliani]

And this isn't even fixable by Crystal's formatter, the point of which is to have one undisputed way to write things.

There is no such thing in programming :-) For instance, there are disputes already towards the use of unless and if at the end of the line:

if condition
  something
end

vs

something if condition

To me, shortening the number of lines (without breaking readability) is an important goal. When compared to Java, that was one of the main advantages of Ruby: no visual noise. And like @spuun mentions, annotations stacking up does introduce some noise to the general structure of the program.

[beta-ziliani]

Regarding the examples shown by @straight-shoota , to me the first example benefits clearly from this:
https://gist.github.com/beta-ziliani/f1f73f14f19763ad02925e12ad397696

But again, it might be my biased love to reducing the number of LOCs.

[vlazar]

We should allow multiple annotations on the same line:

@[Nullable] @[Unique] @[Default(0)] @[Type(smallint)]
property my_key

There is no need to expand the syntax of a single annotation node to allow this.

The fact that annotation in Crystal requires extra [] makes me a little bit sad. Otherwise combining annotations could be much cleaner:

@Nullable @Unique @Default(0) @Type(smallint)
property my_key

Or with commas

@Nullable, @Unique, @Default(0), @Type(smallint)
property my_key

[vlazar]

I've found the ideal solution:

🔮Nullable 🔮Unique 🔮Default(0) 🔮Type(smallint)
property my_key

[asterite]

I don't think we should design the language around reducing LOC

[vlazar]

I don't think we should design the language around reducing LOC

Designing for that? Absolutely no! But there are multiline/single line options already to chose from. For example this looks very clean and natural to me in both forms:

# multiline
getter a
getter b
getter c

# single line
getter a, b, c

Annotation syntax is less clean as it is and with multiple annotations allowed on the same line it looks even less clean to me (relative to other Crystal code).

[beta-ziliani]

I don't think we should design the language around reducing LOC

No, but Crystal is designed to be clean, and this is what's this is about (not saying everyone should agree on what clear is in this particular topic). To me, shortening the lines it takes for annotation gives more space to understand the structure of the class, and that leads to cleaner code. But it should not be confused with the opposite "cleaner code -> fewer LOCs"; that's not what I mean.

[rymiel]

I don't think we should design the language around reducing LOC

If that was the case, #9080 would have been accepted by now

[straight-shoota]

The fact that annotation in Crystal requires extra [] makes me a little bit sad.

Yeah, I don't think they're technically necessary. Annotations are unambiguously determinated by parentheses (or whitespace if there are no arguments). Maybe we can remove them or make them optional? 🤔

Then instead of @[Nullable]; @[Unique]; @[Default(0)]; @[Type(smallint)] you could write @Nullable; @Unique; @Default(0); @Type(smallint) which is probably a bit cleaner. 🤷

[rymiel]

I don't think they're technically necessary

Well, what was the reasoning behind this change from 7 years ago? #195

[beta-ziliani]

BTW, let me add that annotations are akin to C#'s attributes, and in C# you can have multiple of them.

[asterite]

Right! I think our annotations were inspires by those or Java's.

To be honest, I thought it was already possible to specify multiple annotations like that.

[vlazar]

Right! I think our annotations were inspires by those or Java's.

@asterite Can you shed some light why they were using closer to Java syntax from the beginning (@:Name) but then changed to @[Name] in #195?
Now they look exactly like mix of Java + C# :)

[asterite]

I can barely remember what I did last week, I can't remember why we changed the syntax 7 years ago. I think the old syntax looks very weird.

[vlazar]

Reading more C# attributes examples I like it's syntax where you can combine multiple attributes in different ways like:

• [In][Out]
• [In, Out]

I also like that without @ both of these ways look cleaner, especially the first one (or multiline). Also because @ is already used for variables in Crystal.

@[Nullable] @[Unique] @[Default(0)] @[Type(smallint)]
property my_key

@[Nullable]
@[Unique]
@[Default(0)]
@[Type(smallint)]
property my_key

vs

[Nullable] [Unique] [Default(0)] [Type(smallint)]
property my_key

[Nullable]
[Unique]
[Default(0)]
[Type(smallint)]
property my_key

[Blacksmoke16]

My 2 cents, I think it would be worth doing. It's worth pointing out that this new syntax isn't going to replace the old syntax. I.e. I don't think anyone is saying that once/if this feature is added that you must use it. Although having the ability to use it would be helpful.

It would allow you to group related annotations by line while still being able to use multiple lines for distinct groups. For example If you had an ORM you could imagine something like:

@[Column]
@[Id]
@[GeneratedValue("AUTO")]
getter! id : Int64

with this feature you could rewrite it to:

@[Column]
@[Id, GeneratedValue("AUTO")]
getter! id : Int64

So you end up saving a line and also make things more readable by allowing multiple related annotations on each line when it makes sense.

[beta-ziliani]

@vlazar note that [Deprecated] is a list with the attribute Deprecated, so the C# syntax couldn't fly here.

[HertzDevil]

One thing not mentioned is the ability to ignore the newline between the annotation and the annotated node. This makes some things possible:

def fact(x)
  if @[Likely] x > 1
    x * fact(x - 1)
  else
    1
  end
end

Contrast with C++:

int fact(int x) {
  if (x > 1) [[likely]] {
    return x * fact(x - 1);
  else {
    return 1;
  }
}

Crystal's formatter, the point of which is to have one undisputed way to write things

If we do allow commas within @[], it is very likely those nodes would not be parsed as multiple Annotation nodes but as some other new composite AST node; the "undisputed" way to write things would continue to preserve both Annotations and those composites, since they are different nodes. This on the other hand makes some kinds of macros more complicated (e.g. when introspection on ClassDefs is eventually supported), in the same way macros for regular code need to account for Expressions's. That is another reason I believe we should not change the abstract syntax of annotations when a slight relaxation to allow spaces in addition to newlines between adjacent annotations will do the job.

[oprypin]

@HertzDevil I think your concern is very easy to resolve. Just have the compiler convert comma-separated annotations into multiple annotation nodes, as syntax sugar at very early stages of compilation.

[qualterz]

[asterite]

@qualterz great observation! That's exactly why we chose that syntax. It would be, at least partially 😉, familiar to Java and C# programmers

[HertzDevil]

Fun fact: Ruby RBS chose %a{...}.