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

Regression in scalafix v0.11.0 when using language.postfixOps #1794

Closed
RCMartins opened this issue Jun 18, 2023 · 2 comments · Fixed by #1809
Closed

Regression in scalafix v0.11.0 when using language.postfixOps #1794

RCMartins opened this issue Jun 18, 2023 · 2 comments · Fixed by #1809

Comments

@RCMartins
Copy link

RCMartins commented Jun 18, 2023

I think I found a regression in scalafix v0.11.0

If you have this rule:

  override def fix(implicit doc: SemanticDocument): Patch = {
    doc.tree.collect { tree =>
      println(s"structure: ${tree.structure}")
      println(s"syntax: ${tree.syntax}")
      println("-" * 40)
    }
    Patch.empty
  }

And this test file:

import scala.language.postfixOps

object Scalafixtest {
  def hexChar(ch: Int): Char =
    (if (ch < 10) '0' else 'A' - 10) + ch toChar
}

The .syntax in one of the inner ApplyInfix terms is wrong:

structure: Term.ApplyInfix(
  Term.If(
    Term.ApplyInfix(
      Term.Name("ch"),
      Term.Name("<"),
      Type.ArgClause(List()),
      Term.ArgClause(List(Lit.Int(10)), None)
    ),
    Lit.Char(0),
    Term.ApplyInfix(
      Lit.Char(A),
      Term.Name("-"),
      Type.ArgClause(List()),
      Term.ArgClause(List(Lit.Int(10)), None)
    ),
    List()
  ),
  Term.Name("+"),
  Type.ArgClause(List()),
  Term.ArgClause(List(Term.Name("ch")), None)
)
syntax: if (ch < 10) '0' else 'A' - 10) + ch
----------------------------------------

Note that the syntax is missing ( at the beginning of the expression. So the .syntax is returning invalid Scala code. If you create a Patch.replaceTree with this term it will likely produce wrong code.
This issue does not happen in scalafix 0.10.4, so it looks like a regression.


I created a repo that reproduces this issue. It has 2 branches, the main branch with scalafix v0.10.4 where the test passes using sbt tests/test, and the bug branch with v0.11.0 where the test fails.

@bjaglin
Copy link
Collaborator

bjaglin commented Jun 30, 2023

Thanks for the bug report! Let's follow up in scalameta/scalameta#3219

@bjaglin bjaglin linked a pull request Jul 4, 2023 that will close this issue
@bjaglin
Copy link
Collaborator

bjaglin commented Jul 4, 2023

Until v0.11.1 is tagged (no ETA at the moment), snapshots are available to fix that regression.

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 a pull request may close this issue.

2 participants