Add `scala.annotation.MainAnnotation` #14558

nicolasstucki · 2022-02-24T14:48:09Z

The first part of #13727. In the following PR we will include the new @main annotation as an experimental @newMain annotation.

Initially based on https://github.com/lampepfl/dotty/blob/main/tests/pos/main-method-scheme-class-based.scala

nicolasstucki · 2022-02-25T16:55:12Z

library/src/scala/annotation/MainAnnotation.scala

+   *  @param info The information about the command (name, documentation and info about parameters)
+   *  @param args The command line arguments
+   */
+  def command(info: CommandInfo, args: Array[String]): Command[Parser, Result]


I'm considering removing this method from the API but still require MainAnnotation implementations to define it.

This would provide extra flexibility which would make it possible to define it as a macro. The following example is currently impossible but could work with this change.

class myMacroAnnot extends MainAnnotation: type Parser[T] = ... type Result = ... inline def command(inline info: CommandInfo, args: Array[String]): Command[Parser, Result] = ...

The same could apply to argGetter and varargGetter. There we would only need the type parameter and the index parameter and the default option parameter. The implicit parser is not really needed as we could use summonInline or the macro equivalent to conditionally try to summon parsers.

inline def argGetter[T](inline idx: Int, inline defaultArgument: Option[() => T]): () => T = { ... summonInline[Parser[T]] ... }

I could make this change in a following PR and discuss the tradeoff there.

nicolasstucki · 2022-02-28T10:03:23Z

library/src/scala/annotation/MainAnnotation.scala

+     *
+     *  @param program A function containing the call to the main method and instantiation of its arguments
+     */
+    def run(program: () => Result): Unit


Not sure how to restrict the main method to return the Result type when Result =:= Unit as any type can be coerced into a Unit.

Maybe it should be

def run[R](program: () => R)(using R <:< Result): Unit

But then we get error messages like

Cannot prove that Int <:< Unit.

Could also define something like this

def run[R](program: () => R)(using ResultType[R]): Unit @implicitNotFound("The result type of the main method must of type ${Result}") class ResultType[T](using T <:< Result) object ResultType: given [T](using T <:< Result): ResultType[T] = new ResultType

and fail with

|The result type of the main method must of type Unit. |I found: | | cmd.ResultType.given_ResultType_T[Int](/* missing */summon[Int <:< Unit]) | |But no implicit values were found that match type Int <:< Unit.

Alternatively, it could be implemented as a macro and the type could be checked before generating the code. This seems a bit overkill.

The current error message a type Result = Int is also not ideal.

|Found: () => Unit |Required: () => Int

Not sure how to restrict the main method to return the Result type when Result =:= Unit as any type can be coerced into a Unit.

-Ywarn-value-discard

julienrf

Thank you for submitting this PR!

However, I must say that I am a bit skeptical about this idea.

It’s nice to improve the behavior of @main as you did in @newMain, but I am not sure we should commit to letting the users define their own main annotations. Also, your design does not support conditional arguments.

Could you please compare implementing a custom main annotation for IOApp or decline vs the status quo?

julienrf · 2022-02-28T14:27:58Z

library/src/scala/annotation/MainAnnotation.scala

+    val isVarargs: Boolean,
+    val documentation: String,
+    val annotations: Seq[ParameterAnnotation],
+  )


Should we override equals and hashCode to get structural comparisons? (And same for other classes defined here)

Also, should we make the primary constructor private, to support possible evolutions in the future?

Should we override equals and hashCode to get structural comparisons? (And same for other classes defined here)

That is a good idea

Also, should we make the primary constructor private, to support possible evolutions in the future?

We should

julienrf · 2022-02-28T14:52:05Z

library/src/scala/annotation/MainAnnotation.scala

+  )
+
+  /** Marker trait for annotations that will be included in the ParameterInfo annotations. */
+  trait ParameterAnnotation extends StaticAnnotation


Should we just use StaticAnnotation instead of ParameterAnnotation?

This is a way to filter the annotation that need to be reified and instantiated at runtime

julienrf · 2022-02-28T14:54:42Z

library/src/scala/annotation/MainAnnotation.scala

+     *
+     *  @param program A function containing the call to the main method and instantiation of its arguments
+     */
+    def run(program: () => Result): Unit


Not sure how to restrict the main method to return the Result type when Result =:= Unit as any type can be coerced into a Unit.

-Ywarn-value-discard

nicolasstucki · 2022-02-28T15:15:58Z

Also, your design does not support conditional arguments.

What do you mean by conditional arguments? Could you give an example?

julienrf · 2022-02-28T15:34:10Z

Examples: you can pass --foo only if --bar was present, or you can either pass --foo or --bar, but not both.

nicolasstucki · 2022-02-28T16:03:08Z

Also, your design does not support conditional arguments.

What do you mean by conditional arguments? Could you give an example?

Examples: you can pass --foo only if --bar was present, or you can either pass --foo or --bar, but not both.

Seems trivial to support a main annotation with those kinds of semantics. Here is an example

def command(info: CommandInfo, args: Array[String]): Command[Parser, Result] =
  if args.contains("--foo") && !args.contains("--bar") then 
   println("--foo can only be used if --bar is preset")
   System.exist(0)
  ...

Less static version can be done with annotations which can be extracted form the CommandInfo

@myMain def f(@dependsOn("foo") foo: Boolean, bar: Boolean): Unit = ...

julienrf · 2022-02-28T16:06:51Z

Less static version can be done with annotations which can be extracted form the CommandInfo
@myMain def f(@dependsOn("foo") foo: Boolean, bar: Boolean): Unit = ...

Yes, I was thinking of that use case. What I wanted to say is that that use case is not currently supported by @newMain (ie, there is no @dependsOn annotation). Users could add that annotation and the processing logic, but what would be the benefits over doing the same on a plain old @main def main(args: String*) method?

nicolasstucki · 2022-03-01T06:10:49Z

newMain is just an example in this PR. It is far from the definitive version. That will be a discussion for the following PR.

bishabosha · 2022-03-01T14:23:44Z

Yes, I was thinking of that use case. What I wanted to say is that that use case is not currently supported by @newMain (ie, there is no @dependsOn annotation). Users could add that annotation and the processing logic, but what would be the benefits over doing the same on a plain old @main def main(args: String*) method?

the idea would be that a library would do the processing instead of the user

odersky

I won't have the time to review the code in detail I am afraid. I agree with the general direction, though.

anatoliykmetyuk

Otherwise LGTM

tests/run/main-annotation-example.scala

anatoliykmetyuk · 2022-04-01T13:31:40Z

tests/run/main-annotation-homemade-annot-1.scala

+  // This is a toy example, it only works with positional args
+  def command(info: CommandInfo, args: Array[String]): Command[Parser, Result] =
+    new Command[Parser, Result]:
+      override def argGetter[T](idx: Int, defaultArgument: Option[() => T])(using p: Parser[T]): () => T =


I wonder if introducing the command function that handles argument parsing makes the Parser type redundant. Parser has been quite weak since its inception since we didn't want to overcomplicate command line parsing and reasoning that people with more advanced use cases can fall back to defining the main method manually.

Could you elaborate on how to remove the Parser?

I believe those getters the user has to override and the implicit Parsers server the same purpose – that is transform the raw String arguments into values of a certain type. We could do with just one of those, thus reducing the complexity. E.g. we could ditch the getter methods in favor of Parsers being present in the implicit scope.

tests/run/main-annotation-homemade-annot-1.scala

tests/run/main-annotation-homemade-annot-2.scala

tests/run/main-annotation-homemade-annot-3.scala

compiler/src/dotty/tools/dotc/ast/MainProxies.scala

nicolasstucki · 2022-04-01T16:05:47Z

compiler/src/dotty/tools/dotc/ast/MainProxies.scala

-      var lines: Seq[String] = raw.trim.split('\n').toSeq
-      lines = lines.map(l => l.substring(skipLineLead(l, -1), l.length).trim)
+      var lines: Seq[String] = raw.trim.nn.split('\n').nn.toSeq
+      lines = lines.map(l => l.substring(skipLineLead(l, -1), l.length).nn.trim.nn)


@olhotak here is a real-world use case where we need too many .nn

anatoliykmetyuk · 2022-04-04T11:22:38Z

library/src/scala/annotation/MainAnnotation.scala

+   *  @param info The information about the command (name, documentation and info about parameters)
+   *  @param args The command line arguments
+   */
+  def command(info: CommandInfo, args: Array[String]): Command[?, ?]


I believe the result type needs to be defined on the annotation level, not the command level. This way, we force the user to define the command type that can handle the result type known to be used with this particular annotation.

Why do we need the command abstraction anyway? Why can we not just require the user to implement a single run method to define the entire annotation? That is, the entire new annotation is fully described by a single function, of a signature similar to def run(program: () => Result, rawArgs: Array[String], argInfos: CommandInfo): Unit. Do you have any use cases that cannot be covered by an annotation defined by a single function?

Parsing logic of the arguments can be fully abstracted into the compiler's desugaring and depend on the implicit Parsers defined with the annotation, following a common-sense logic to handle default parameters and varargs.

How would you get the Parser instances in that encoding? The point of having the def argGetter[T]... is to have the type and be able to summon the Parser[T].

That would happen at the same place in desugaring where argGetter[T] is called. The difference is that instead of argGetter[T], summon[Parser[T]] will be called, and also there will be supporting logic that would handle which arguments from Array[String] get processed with which summoned parsers. This logic will also take into account different styles of specifying parameters from the command line in a common-sense way: e.g. with or without dash syntax, with the possibility of params with default values omitted etc.

If the user needs something more complicated than that common sense, they can always fall back to the good-old def main.

Currently Parser does not define any API. We would not be able to call any method on it to parse the argument.

We also do not know which argument to parse as the Array[String] may need some preprocessing.

I'll investigate this possibility

@anatoliykmetyuk I simplified the interface a significant amount thanks to your suggestions. Now we do not have the Command class anymore, everything is in MainAnnotation. I still keep the argGetter/varargsGetter as these are necessary to handle errors while parsing. The current design is as expressive as the old one.

Looks much better! See my review comments below.

Also, can you elaborate how argGetter/varargGetter can be used for error handling the way the summoned parser can't? From the MainProxies doc, parsing of arguments:

* val args0: () => S = annotation.argGetter[S](info.parameters(0), cmd(0), None)

Do you believe there is something this construct can express that the following can't regarding error handling?

* val args0: () => S = summon[Parser[S]].parse(info.parameters(0), cmd(0), None)

The newMain uses this to try to parse all arguments before failing. In case of a failure, the error is reported and the program continues trying to parse the following arguments. Then the run does not evaluate the program if some parsing errors happened.

See example:

argGetter/varargsGetter/run: https://github.com/lampepfl/dotty/pull/14841/files#diff-73b166621bb3162b59d2e39763a5883b7d44966e7e7959b260417f9a77d2be24R90-R104

Parsing and error handling: https://github.com/lampepfl/dotty/pull/14841/files#diff-73b166621bb3162b59d2e39763a5883b7d44966e7e7959b260417f9a77d2be24R173-R184

nicolasstucki · 2022-04-06T16:36:24Z

library/src/scala/annotation/MainAnnotation.scala

+   *
+   *  @param program A function containing the call to the main method and instantiation of its arguments
+   */
+  def run(program: () => Result): Unit


I wonder if we should pass the info to this method as well

def run(info: Info, program: () => Result): Unit

It might be useful for error reporting if the program fails

If the user really needs it, they can always set a private mutable variable from command.

Yes, they can.

tests/run/main-annotation-newMain.scala

compiler/src/dotty/tools/dotc/ast/MainProxies.scala

tests/run/main-annotation-homemade-annot-1.scala

See `docs/_docs/reference/experimental/main-annotation.md`

Remove the `Command` class and place the `argGetter`, `varargsGetter` and `run` methods directly in the `MainAnnotation` interface. Now `command` pre-processes the arguments which clearly states which strings will be used for each argument. This simplifies the implementation of the `MainAnnotation` methods.

nicolasstucki · 2022-04-14T13:00:56Z

Rebased and updated MiMaFilters.

bjornregnell · 2022-04-18T15:37:53Z

docs/_docs/reference/experimental/main-annotation.md

+      name = "foo.main",
+      documentation = "Sum all the numbers",
+      parameters = Seq(
+        new Parameter("first", "scala.Int", hasDefault=false, isVarargs=false, "Fist number to sum", Seq()),


Typo here: Should be "First" not "Fist"

nicolasstucki force-pushed the add-scala.annotation.MainAnnotation branch from 2a9d894 to edc34e8 Compare February 24, 2022 14:57

nicolasstucki mentioned this pull request Feb 24, 2022

Generalize main annotations #13727

Closed

nicolasstucki force-pushed the add-scala.annotation.MainAnnotation branch from c0136b7 to c040077 Compare February 25, 2022 10:19

nicolasstucki commented Feb 25, 2022

View reviewed changes

nicolasstucki self-assigned this Feb 28, 2022

nicolasstucki commented Feb 28, 2022

View reviewed changes

nicolasstucki marked this pull request as ready for review February 28, 2022 10:05

nicolasstucki requested a review from smarter February 28, 2022 10:05

nicolasstucki assigned smarter and unassigned nicolasstucki Feb 28, 2022

nicolasstucki requested a review from odersky February 28, 2022 10:06

nicolasstucki assigned odersky Feb 28, 2022

julienrf reviewed Feb 28, 2022

View reviewed changes

odersky reviewed Mar 3, 2022

View reviewed changes

odersky removed their assignment Mar 3, 2022

smarter mentioned this pull request Mar 13, 2022

Should report argument is missing instead of no implicit argument was found ? #14680

Open

prolativ requested a review from anatoliykmetyuk March 14, 2022 14:27

nicolasstucki assigned anatoliykmetyuk and unassigned smarter Mar 15, 2022

anatoliykmetyuk reviewed Apr 1, 2022

View reviewed changes

anatoliykmetyuk assigned nicolasstucki and unassigned anatoliykmetyuk Apr 1, 2022

nicolasstucki force-pushed the add-scala.annotation.MainAnnotation branch from 9bd6708 to deb7d0b Compare April 1, 2022 15:56

nicolasstucki commented Apr 1, 2022

View reviewed changes

nicolasstucki requested a review from anatoliykmetyuk April 4, 2022 10:45

nicolasstucki assigned anatoliykmetyuk and unassigned nicolasstucki Apr 4, 2022

anatoliykmetyuk reviewed Apr 4, 2022

View reviewed changes

nicolasstucki mentioned this pull request Apr 4, 2022

Implement new main annotation #14841

Merged

nicolasstucki force-pushed the add-scala.annotation.MainAnnotation branch 5 times, most recently from eb6148f to ce0b59a Compare April 6, 2022 07:50

nicolasstucki requested a review from anatoliykmetyuk April 6, 2022 12:18

nicolasstucki commented Apr 6, 2022

View reviewed changes

anatoliykmetyuk reviewed Apr 7, 2022

View reviewed changes

tests/run/main-annotation-newMain.scala Show resolved Hide resolved

compiler/src/dotty/tools/dotc/ast/MainProxies.scala Outdated Show resolved Hide resolved

tests/run/main-annotation-homemade-annot-1.scala Show resolved Hide resolved

nicolasstucki force-pushed the add-scala.annotation.MainAnnotation branch 2 times, most recently from 2a385ed to 8e37659 Compare April 11, 2022 11:51

nicolasstucki added the release-notes Should be mentioned in the release notes label Apr 13, 2022

anatoliykmetyuk approved these changes Apr 14, 2022

View reviewed changes

anatoliykmetyuk assigned nicolasstucki and unassigned anatoliykmetyuk Apr 14, 2022

timotheeandres and others added 2 commits April 14, 2022 14:54

Add scala.annotation.MainAnnotation

e852aa7

See `docs/_docs/reference/experimental/main-annotation.md`

nicolasstucki force-pushed the add-scala.annotation.MainAnnotation branch from 8e37659 to 813e059 Compare April 14, 2022 12:59

nicolasstucki enabled auto-merge April 14, 2022 13:01

nicolasstucki merged commit 1bd7c77 into scala:main Apr 14, 2022

nicolasstucki deleted the add-scala.annotation.MainAnnotation branch April 14, 2022 14:39

bjornregnell reviewed Apr 18, 2022

View reviewed changes

Kordyjan added this to the 3.2.0 milestone Aug 1, 2023

Add scala.annotation.MainAnnotation #14558

Add scala.annotation.MainAnnotation #14558

Conversation

nicolasstucki commented Feb 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolasstucki Feb 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julienrf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolasstucki commented Feb 28, 2022

julienrf commented Feb 28, 2022

nicolasstucki commented Feb 28, 2022

julienrf commented Feb 28, 2022

nicolasstucki commented Mar 1, 2022

bishabosha commented Mar 1, 2022

odersky left a comment

Choose a reason for hiding this comment

anatoliykmetyuk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anatoliykmetyuk Apr 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anatoliykmetyuk Apr 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anatoliykmetyuk Apr 7, 2022 • edited Loading

Choose a reason for hiding this comment

nicolasstucki Apr 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolasstucki commented Apr 14, 2022

bjornregnell Apr 18, 2022 • edited Loading

Choose a reason for hiding this comment

Add `scala.annotation.MainAnnotation` #14558

Add `scala.annotation.MainAnnotation` #14558

nicolasstucki commented Feb 24, 2022 •

edited

Loading

nicolasstucki Feb 28, 2022 •

edited

Loading

anatoliykmetyuk Apr 4, 2022 •

edited

Loading

anatoliykmetyuk Apr 4, 2022 •

edited

Loading

anatoliykmetyuk Apr 7, 2022 •

edited

Loading

nicolasstucki Apr 11, 2022 •

edited

Loading

bjornregnell Apr 18, 2022 •

edited

Loading