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

Disambiguate akka-http when optional parameter lists are specified #250

Merged
merged 5 commits into from
Apr 25, 2019

Conversation

blast-hardcheese
Copy link
Member

Resolves: #249

  • Resolves syntactic ambiguity for akka-http parameter matching
  • Additionally, for optional type: array parameters, adds a filter(_.nonEmpty), removing the possibility of Some(Nil)
  • Also fixing optional list parameter matching compiler error in http4s that was exposed when adding the tests for akka-http.

Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

  • I acknowledge that all my contributions will be made under the project's license.

@kelnos
Copy link
Member

kelnos commented Apr 25, 2019

Is there no better way to do this? All the .apply makes the generated code harder to read and pretty ugly, IMO :(

@blast-hardcheese
Copy link
Member Author

I'm working against scalameta/scalameta#1634, unfortunately.

path("foo") {
  complete(200)
}

gets translated into

Term.Apply(
  Term.Apply(Term.Name("path"), Lit.String("foo")),
  Term.Block(List(Term.Apply(Term.Name("complete"), Lit.Int(200))))
)

when generated, but

Term.Apply(
  Term.Apply(Term.Name("path"), Lit.String("foo")),
  Term.Apply(Term.Name("complete"), Lit.Int(200))
)

when parsed. I tried a bunch of different ways to avoid this, but was ultimately unsuccessful. Now, however, scalac treats parameter(Symbol("bar").as[String].*).map(Option.apply _)( bar => ... ) in such a way that we lose type information on the Option.apply _.

An alternate strategy here could be fixing ${tpe} in Option.apply[???] _ which runs the risk of the types slipping between those two points (for instsance if ${tpe} isn't the base type, but the wrapped type, we'd end up with tpe = Option[List[Long]], Option[$tpe], ending up with Option[Option[List[Long]]]).

Let me try that instead.

@blast-hardcheese
Copy link
Member Author

blast-hardcheese commented Apr 25, 2019

@kelnos OK, I was able to constrain it to just the offending

parameter(Symbol(${arg}).as[${tpe}].*).map(xs => Option(xs)).apply

Copy link
Member

@kelnos kelnos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, that's much nicer! :)

@blast-hardcheese blast-hardcheese merged commit 1feb0d1 into guardrail-dev:master Apr 25, 2019
@blast-hardcheese blast-hardcheese deleted the issue-249 branch April 25, 2019 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

in:query and type:array results in "missing parameter type"
2 participants