scalacenter/scalafix

This room is no longer active, please join the rest of the scalameta community on https://discord.gg/8AHaqGx3Qj

Brice Jaglin

@bjaglin

Oops, that's scalacenter/scalafix#1418... That also means scaluzzi 0.1.19 or any rule built against Scalafix 0.9.28 making use of metaconfig.generic.Surface will not run on Scalafix<0.9.28, which is as concerning, if not more.

Brice Jaglin

@bjaglin

I am not sure what's going on here, given the linked overload in the NoSuchMethodError - does https://github.com/scalameta/metaconfig/commit/df7731d981619837e88eb7c9e48913bc58f53622#diff-bb8e899325e39d9a28fc668670cf053128dd777a526390667b26fd7f83ceb047R15 get inlined? Interestingly, MiMa was added in that commit, but I guess it does not check for this kind of forward-compatibility?

Ólafur Páll Geirsson

@olafurpg

@bjaglin adding a new method is backwards compatible, but forwards incompatible. The scala-library jar is the only API that I'm aware of in the Scala community that guarantees forward compatibility, because it's very difficult to evolve a library without adding new methods that break forwards compatibility

As long as the latest Scalafix uses the latest metaconfig, then it should be able to support older versions of Scalafix rules (regardless of what version of metaconfig they use)

it's not too important to allow "latest version of custom rule works with older versions of Scalafix"

it's desirable, but impractical

Brice Jaglin

@bjaglin

@olafurpg thanks, I see! In that particular case, could we tweak https://github.com/scalameta/metaconfig/commit/df7731d981619837e88eb7c9e48913bc58f53622#diff-335f40226027fbc0a85b9fc7a4b4fc9f79df8ffba3ec34519f057295a0a7a00fR210 to avoid calling the new arity-2 constructor if classAnnotations is empty? it looks like a quick fix.

Brice Jaglin

@bjaglin

Also, I am wondering if scalafix-interfaces shouldn't handle eviction betters: metaconfig.generic.Surface was already loaded by the scalafix-cli classloader (with an old version) to parse the configuration, so the one in the --toolclasspath(newer, required by the classloaded community rule) is effectively ignored. Assuming security is not a concern here, we could let toolclasspath evict dependencies from the internal classpath when using the high-level withToolClasspathoverloads using coursier coordinates? Some IDE/build-tool integrations might lag behind community rules (see https://github.com/evis/scalafix-maven-plugin/commits/master for example). Related note: https://github.com/scalacenter/scalafix/commit/3694af653dde8c6c05a2af5051540a3f7f7870a6#diff-26de4adbd0ac932a910d1ec9c7b8a8784e07dc15a96313961a927194d96eef00R57-R61.

Ólafur Páll Geirsson

@olafurpg

Avoiding the 2-arity constructor when possible sounds good to me. Although it's unfortunate so much investment is needed to work around this bincompat issue

I'm on the fence about letting the tool classpath override the scalafix root classpath. Does that mean a community rule could override the internal classpath to use an older version of metaconfig/scalafix?

My intuition had been that community rules are more likely to lag behind on Scalafix versions instead of the other way around

Brice Jaglin

@bjaglin

Does that mean a community rule could override the internal classpath to use an older version of metaconfig/scalafix?

I was thinking the most recent one could evict the other (coursier fetch logic)

Although it's unfortunate so much investment is needed to work around this bincompat issue

macros make harder a problem already hard enough :)

Ólafur Páll Geirsson

@olafurpg

True, but without macros we would have other kinds of problems related to outdated boilerplate or buggy code generation scripts :)

does scalafix-interfaces download scalafix AND the custom rules in the same coursier resolution?

the older version is only evicted within a single resolution, and my understanding is that custom rules are resolved via a separate resolution (but I may be missing something)

Brice Jaglin

@bjaglin

Even though it's indeed 2 separate resolutions (the first one in scalafix-interfaces, and the second one in scalafix-cli), I naively thought the filtering strategy used to sandbox scalafix-cli behind scalafix-interfaces could be adapted so that classloaded rules share only scalafix.v1.* (or something like that) with the scalafix-cli classloader. But after looking at the code, I realized some scalameta/metaconfig types are exposed through scalafix.v1.*, so that lack of clear boundaries makes it impossible to get both sandboxing AND class unicity across classloaders.

Brice Jaglin

@bjaglin

Since the toolclasspath is most often known beforehand, we could add some methods in scalafix-interfaces that classload scalafix-cli AND custom rules in one go though. The problem I see is that the CLI itself wouldn't benefit from that, so for the same scalafix-cli/interfaces version, behavior would be different between a pure CLI invocation and one through a build tool / IDE.

Dario Abdulrehman

@dabd

Can you run scalafix rules on .sbt files? I wanted to apply https://github.com/liancheng/scalafix-organize-imports on these files as well.

Brice Jaglin

@bjaglin

@dabd only syntactic rules are supported on *.sbt files, so I am afraid you can't run that semantic rule

Dario Abdulrehman

@dabd

thanks @bjaglin

Michael Thomas

@Michaelt293

I'm wondering, is there a good way to go from a Symbol to the scala meta AST for that symbol?

Ólafur Páll Geirsson

@olafurpg

@Michaelt293 a symbol can be loaded from the classpath where you don’t have access to the sources. What would the AST contain?

You can render the signature of a symbol into Scala syntax, which you could parse

but you wouldn’t have the implementation then

Michael Thomas

@Michaelt293

Thanks @olafurpg , I understand that it might be a bit of a strange thing to want to do. A simplified example of what I'm trying to do is something similar to this-

  trait Level {
    def level = 1
  }

  class Example extends Level

Rewrite as-

  class Example {
    def level = 1
  }

To do this, I would need to have access to the AST for Level and Level could be defined in another file. If Level was defined in a dependency, all bets are off and I would not be able to get the AST.

Ólafur Páll Geirsson

@olafurpg

@Michaelt293 there's no straightforward way to accomplish that with the current API

One backwards way to achieve it would be to run scalafix twice, where you collect the ASTs in the first pass and persist them somehow in Rule.afterComplete() https://github.com/scalacenter/scalafix/blob/1c0ba35cc71f28551f98ea607d47f4db7147d802/scalafix-core/src/main/scala/scalafix/v1/Rule.scala#L46

but Scalafix alone doesn't really have sufficient APIs to implement this anways

when you inline code from a separate file then you need to pretty-print the original source in a way so that all the identifiers resolve to the same symbols as in the original file

ExplicitResultTypes has logic like that but you can look at the implementation to see that it's not that trivial to do correctly

I estimate it's very hard to implement this refactoring if you're aiming for >99% correctness

Brice Jaglin

@bjaglin

If you use sbt-scalafix to implement the 2-rule hack, make sure you opt-out from scalafixCaching, otherwise re-runs will only capture the AST of changed files

Michael Thomas

@Michaelt293

Thanks for the information :)

Albert Meltzer

@kitbellew

@olafurpg @bjaglin i finally decided to try scalafix, and now i need some help. how do i run scalafix-cli (we do not use sbt) with the OrganizeImports external rule? i got scalafix the same way we do with scalafmt (via coursier standalone bootstrap). ideally, the external jar is also downloaded in advance as build servers are behind firewall and might not be able to.

Ólafur Páll Geirsson

@olafurpg

@kitbellew semantic rules like OrganizeImports need access to the classpath of the project and semanticdb files of the sources being fixed. This makes it more complicated to run scalafix from the cli compared to scalafmt.

What build tool are you using?

The classpath of the OrganizeImports rule can be added via —tool-classpath

or you can also add the ruleto the same bootstrap jar as scalafix-cli

Albert Meltzer

@kitbellew

@olafurpg how can i ask coursier to pull in the additional jar into the bootstrap for base scalafix?

and say i did add it, or packaged the external rule into a separate jar and listed it via --tool-classpath; how do i get scalafix to invoke it? all examples are by adding a dependency in sbt via rule@x:y:z but this approach is different

ah, and the build tool is maven

Ólafur Páll Geirsson

@olafurpg

@kitbellew have you tried this plugin? https://github.com/evis/scalafix-maven-plugin

You can add multiple dependencies to the same coursier bootstrap command. You may need to specify the main class

Once the external rule is available on the same classpath as scalafix-cli (or tool classpath) then you can reference the rule via name

Ólafur Páll Geirsson

@olafurpg

The rule@ syntax is an sbt-only feature, which is prinarily intended for one-off refactorings

Albert Meltzer

@kitbellew

@olafurpg thanks, tried the plugin. will need a fix because it currently supports only one version of scala. evis/scalafix-maven-plugin#12

Krzysiek Bochenek

@kpbochenek

Hi, I want to forbid certain implicit(import?) from using in my codebase and I was wondering is it something scalafix can do? Is there a rule that potentially could be used? Or maybe there is better dedicated tool for that?

Tomasz Godzik

@tgodzik

@kpbochenek I think it should be possible, but you would need to write a custom linting rule.

Krzysiek Bochenek

@kpbochenek

@tgodzik nice! do you have any examples where I could start looking at this? are there any custom linting rules already doing something similar?

Where communities thrive

People

Repo info

Activity