Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ScalaGenerator should follow Scala Style Guide in generated code #5422

Closed
3 tasks done
lukaseder opened this issue Jul 19, 2016 · 7 comments
Closed
3 tasks done

ScalaGenerator should follow Scala Style Guide in generated code #5422

lukaseder opened this issue Jul 19, 2016 · 7 comments

Comments

@lukaseder
Copy link
Member

lukaseder commented Jul 19, 2016

Currently, the ScalaGenerator generates:

identifier : Type = value

When it should generate:

identifier: Type = value

All improvements:

  • Avoid whitespace before :
  • Avoid return where possible
  • Use expression method bodies def x(): T = expr (single line) rather than def x(): T = { expr } (multi line)

See:

@lukaseder
Copy link
Member Author

Implemented with #5421.

This change is too minor compared to the lines of code adapted. We'll thus not merge this to jOOQ 3.8.x

@lukaseder
Copy link
Member Author

lukaseder commented Jul 19, 2016

Hmm... This seems to introduce regressions related to the disambiguations put in place by jOOQ when adding suffixes. For instance, the following is illegal scala code:

def getClass_: Integer = { ... }

I guess this complicates things unnecessarily, without adding too much value. Will mark as "won't fix"

@er1c
Copy link
Contributor

er1c commented Jul 19, 2016

Just so this note is somewhere, it occurred to me, this can be fixed in scala by wrapping the identifier in backticks if the last character is an underscore _

scala> def getClass_: String = "foo"
<console>:1: error: '=' expected but identifier found.
def getClass_: String = "foo

scala> def `getClass_`: String = "foo"
getClass_: String

I'll try submitting a more complete PR that includes the proper SCALA escaping of the identifier - maybe we can move this back from the "wontfix" bucket with a little bit more work.

Here's the lexical reference that matters the most here: http://www.scala-lang.org/files/archive/spec/2.11/01-lexical-syntax.html

idrest ::= {letter | digit} [‘_’ op]

Operator characters. These consist of all printable ASCII characters \u0020 - \u007F which are in none of the sets above, mathematical symbols (Sm) and other symbols (So).

There are three ways to form an identifier. First, an identifier can start with a letter which can be followed by an arbitrary sequence of letters and digits. This may be followed by underscore ‘_‘ characters and another string composed of either letters and digits or of operator characters. Second, an identifier can start with an operator character followed by an arbitrary sequence of operator characters. The preceding two forms are called plain identifiers. Finally, an identifier may also be formed by an arbitrary string between back-quotes (host systems may impose some restrictions on which strings are legal for identifiers). The identifier then is composed of all characters excluding the backquotes themselves.

So, the only time _: would cause a compile error is when _ is the last character of the "original" identifier string, because it thinks there will be an op coming after it.

Adding the backticks around the identifier should do the trick if the last character is that _

@er1c
Copy link
Contributor

er1c commented Jul 20, 2016

I fucked with this for probably more than I should have today, but I think it's coming together fine: #5431

@lukaseder
Copy link
Member Author

this can be fixed in scala by wrapping the identifier in backticks if the last character is an underscore _

Interesting. But this doesn't mean that the call site will need to quote the identifier too, right?

@er1c
Copy link
Contributor

er1c commented Jul 20, 2016

@lukaseder Correct, the call site does not need to escape the identifier....unless the identifier had whitespace characters in it...you could technically do:

object FooBar {
  def `foo and bar`: String = "foo and bar"
}

FooBar.`foo and bar`

Will yield "foo and bar"

@lukaseder
Copy link
Member Author

Fixing this now in the context of #10191

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants