scalameta icon indicating copy to clipboard operation
scalameta copied to clipboard

newline syntax is not optimal

Open prongs opened this issue 4 years ago • 4 comments

Hi all

We have a piece of code that does some scala code generation. Now, at some point, I had a string that is just "\n" and I wanted to generate a code that looks consistent across all values of this string.

As an example, let's say the string is supposed to be a delimiter and you're supposed to generate a splitter function. Something as simple as below:

object Test extends App {
  import scala.meta._
  import scala.meta.inputs._
  import scala.meta.inputs.Input._
  import scala.meta.internal.trees.Origin
  val delimiters = List("\n", "\t")
  for (delimiter <- delimiters) {
    val code =
      q"""
        def split(s: String) = s.split(${Lit.String(delimiter)})
       """
    println(s"\n----\n$code\n---\n")
  }
}

Now, the code that gets printed for \t looks fine

def split(s: String) = s.split("\t")

But for \n, ScalaMeta has a special behavior

def split(s: String) = s.split("""
""")

What I want, instead is this:

def split(s: String) = s.split("\n")

I spent some time going through the scala meta codebase and found that the reason is this line. The behavior seems to be hard-coded in scala-meta.

I believe there might have been a good reason to do this, but this ends up generating dirty code, especially in the case where the string just contains a single character of newline. This behavior could have been kept restricted to when there is more than one occurrence of the newline character.

Potential Solutions

One potential solution I see is to define my own syntax function (syntax2), the implicit classes/methods around it, and call that. There, I am a little blocked because the class is declared as final. I could of-course copy the whole class, but I'd rather want to avoid that, and go that route only if I don't have any other options.

Another potential solution would be if there were some flags in ScalaMeta pretty-printing where I can control this behavior. There doesn't seem to be such a flag available.

Another potential solution could be to enhance this behavior, let it go in the next release, and then depend on the latest version of ScalaMeta.

prongs avatar Mar 28 '22 12:03 prongs

Thanks for reporting! Isn't this a case of just escaping the \n and \t

Looks like delimeters should be: val delimiters = List("\\n", "\\t") otherwise we are just adding a string with a newline or tab.

tgodzik avatar Mar 28 '22 13:03 tgodzik

No, this is not what I'm looking for

This generates codes like

def split(s: String) = s.split("\\n")
def split(s: String) = s.split("\\t")

neither of which are what I want. \\n in java/scala syntax is two characters: \ (ASCII 92) and n (ASCII 110). Whereas \n is a Single character ASCII-10.

prongs avatar Mar 28 '22 16:03 prongs

Ach, I see! Maybe as a workaround it;s possible to use Lit.Char('\n') and Lit.Char('\t') ? Split should also with with a char.

Not sure how to work around the current behaviour otherwise.

tgodzik avatar Mar 28 '22 16:03 tgodzik

@prongs what is the definition of "dirty" in the context of generated code?

your proposed change might not work consistently for all values, only perhaps across the subset of values that you use.

kitbellew avatar Apr 03 '22 06:04 kitbellew

@kitbellew Why is this marked as "complete"?

To answer your previous question

what is the definition of "dirty" in the context of generated code?

We're using scala meta to generate scala code for our customers. I can't generate

def split(s: String) = s.split("""
""")

for them. This is utterly unreadable for whoever is consuming this generated code.

I'd rather want to generate

def split(s: String) = s.split("\n")

which is so much cleaner.

scalameta give me no lever to toggle this behaviour. So In my opinion, we should not mark this as "Complete".

prongs avatar Jan 03 '23 09:01 prongs

@kitbellew Why is this marked as "complete"?

It's been 9 months without a reply in a situation unlikely to have a resolution.

To answer your previous question

what is the definition of "dirty" in the context of generated code?

We're using scala meta to generate scala code for our customers. I can't generate

def split(s: String) = s.split("""
""")

for them. This is utterly unreadable for whoever is consuming this generated code.

Unfortunately, the prettyprinting library is not designed to generate code which will be considered readable by everyone.

Also, I wouldn't call it "unreadable" (let alone "utterly"). In fact, if someone were to specify

def split(s: String) = s.split("""
// ammonite script reload
@
""")

they would probably find

def split(s: String) = s.split("""\n// ammonite script reload\n@\n""")

utterly unreadable. There's no obvious, clean, universal solution to this. Let us know if you can think of one.

kitbellew avatar Jan 03 '23 09:01 kitbellew

There's no obvious, clean, universal solution to this.

If we think of it as black and white, then, of course, there is no clean solution. A pragmatic approach would be to have some levers/flags in the pretty-printing path, which the user can supply. Keeping the current code paths intact, a new pretty-printing path which can take in these levers/flags.

Of course, I'm speaking more as a user of the library and only understand the pretty-printing paths superficially. So what I suggested above might not even be feasible. You are a better judge, with your understanding of the internals.

prongs avatar Jan 04 '23 05:01 prongs

pretty printing has no flags. all i can recommend is using a formatter, with the right set of flags, after the files had been generated.

kitbellew avatar Jan 04 '23 05:01 kitbellew

Any thoughts around this?

a new pretty-printing path which can take in these levers/flags

prongs avatar Jan 05 '23 09:01 prongs

Any thoughts around this?

a new pretty-printing path which can take in these levers/flags

This looks like a lot of work and some of it will be replicating the work in scalafmt, which already has much large amount of options. This is not something we want to work on, so the recommended way would be to do it in two steps as suggested by @kitbellew This offers a lot more options right from the bat.

tgodzik avatar Jan 05 '23 10:01 tgodzik

Isn't the formatter (scalafmt) also based on scalameta? If scalameta does not have this capability, how will scalafmt have this capability?

prongs avatar Jan 06 '23 06:01 prongs

Isn't the formatter (scalafmt) also based on scalameta? If scalameta does not have this capability, how will scalafmt have this capability?

Scalafmt is a formatter so it really exists for that sole purpose. It does not add pretty printing, but just modifies existing code without changing the AST.

tgodzik avatar Jan 06 '23 07:01 tgodzik