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

create analyzer plugin for 2.13.6 #58

Closed
wants to merge 1 commit into from
Closed

create analyzer plugin for 2.13.6 #58

wants to merge 1 commit into from

Conversation

tek
Copy link
Owner

@tek tek commented Jun 21, 2021

this is not complete yet, just a skeleton with the shapeless record formatter.

turns out the plugin hook is a bit crude, still need to override and coerce types…

@github-actions
Copy link

Test Report (2.12.7)

0 files   -   3  0 suites   - 3   0s ⏱️ -24s
0 tests  - 23  0 ✔️  - 23  0 💤 ±0  0 ❌ ±0 

Results for commit f93f0ae. ± Comparison against base commit 174ed63.

This pull request removes 23 tests.
splain.ErrorsCompat2_13_1Spec ‑ ambiguous implicits
splain.ErrorsSpec ‑ Tuple1
splain.ErrorsSpec ‑ aux type
splain.ErrorsSpec ‑ byname higher order
splain.ErrorsSpec ‑ compact tree printing
splain.ErrorsSpec ‑ deep hole
splain.ErrorsSpec ‑ disambiguate types
splain.ErrorsSpec ‑ found/required type diff
splain.ErrorsSpec ‑ implicit resolution chains
splain.ErrorsSpec ‑ linebreak long infix types
…

@github-actions
Copy link

Test Report (2.12.8)

0 files   -   3  0 suites   - 3   0s ⏱️ -24s
0 tests  - 23  0 ✔️  - 23  0 💤 ±0  0 ❌ ±0 

Results for commit f93f0ae. ± Comparison against base commit 174ed63.

This pull request removes 23 tests.
splain.ErrorsCompat2_13_1Spec ‑ ambiguous implicits
splain.ErrorsSpec ‑ Tuple1
splain.ErrorsSpec ‑ aux type
splain.ErrorsSpec ‑ byname higher order
splain.ErrorsSpec ‑ compact tree printing
splain.ErrorsSpec ‑ deep hole
splain.ErrorsSpec ‑ disambiguate types
splain.ErrorsSpec ‑ found/required type diff
splain.ErrorsSpec ‑ implicit resolution chains
splain.ErrorsSpec ‑ linebreak long infix types
…

@github-actions
Copy link

Test Report (2.12.10)

0 files   -   3  0 suites   - 3   0s ⏱️ -24s
0 tests  - 23  0 ✔️  - 23  0 💤 ±0  0 ❌ ±0 

Results for commit f93f0ae. ± Comparison against base commit 174ed63.

This pull request removes 23 tests.
splain.ErrorsCompat2_13_1Spec ‑ ambiguous implicits
splain.ErrorsSpec ‑ Tuple1
splain.ErrorsSpec ‑ aux type
splain.ErrorsSpec ‑ byname higher order
splain.ErrorsSpec ‑ compact tree printing
splain.ErrorsSpec ‑ deep hole
splain.ErrorsSpec ‑ disambiguate types
splain.ErrorsSpec ‑ found/required type diff
splain.ErrorsSpec ‑ implicit resolution chains
splain.ErrorsSpec ‑ linebreak long infix types
…

@github-actions
Copy link

Test Report (2.12.9)

0 files   -   3  0 suites   - 3   0s ⏱️ -28s
0 tests  - 23  0 ✔️  - 23  0 💤 ±0  0 ❌ ±0 

Results for commit f93f0ae. ± Comparison against base commit 174ed63.

This pull request removes 23 tests.
splain.ErrorsCompat2_13_1Spec ‑ ambiguous implicits
splain.ErrorsSpec ‑ Tuple1
splain.ErrorsSpec ‑ aux type
splain.ErrorsSpec ‑ byname higher order
splain.ErrorsSpec ‑ compact tree printing
splain.ErrorsSpec ‑ deep hole
splain.ErrorsSpec ‑ disambiguate types
splain.ErrorsSpec ‑ found/required type diff
splain.ErrorsSpec ‑ implicit resolution chains
splain.ErrorsSpec ‑ linebreak long infix types
…

@github-actions
Copy link

Test Report (2.12.11)

0 files   -   3  0 suites   - 3   0s ⏱️ -24s
0 tests  - 23  0 ✔️  - 23  0 💤 ±0  0 ❌ ±0 

Results for commit f93f0ae. ± Comparison against base commit 174ed63.

This pull request removes 23 tests.
splain.ErrorsCompat2_13_1Spec ‑ ambiguous implicits
splain.ErrorsSpec ‑ Tuple1
splain.ErrorsSpec ‑ aux type
splain.ErrorsSpec ‑ byname higher order
splain.ErrorsSpec ‑ compact tree printing
splain.ErrorsSpec ‑ deep hole
splain.ErrorsSpec ‑ disambiguate types
splain.ErrorsSpec ‑ found/required type diff
splain.ErrorsSpec ‑ implicit resolution chains
splain.ErrorsSpec ‑ linebreak long infix types
…

@github-actions
Copy link

Test Report (2.12.12)

0 files   -   3  0 suites   - 3   0s ⏱️ -23s
0 tests  - 23  0 ✔️  - 23  0 💤 ±0  0 ❌ ±0 

Results for commit f93f0ae. ± Comparison against base commit 174ed63.

This pull request removes 23 tests.
splain.ErrorsCompat2_13_1Spec ‑ ambiguous implicits
splain.ErrorsSpec ‑ Tuple1
splain.ErrorsSpec ‑ aux type
splain.ErrorsSpec ‑ byname higher order
splain.ErrorsSpec ‑ compact tree printing
splain.ErrorsSpec ‑ deep hole
splain.ErrorsSpec ‑ disambiguate types
splain.ErrorsSpec ‑ found/required type diff
splain.ErrorsSpec ‑ implicit resolution chains
splain.ErrorsSpec ‑ linebreak long infix types
…

@github-actions
Copy link

Test Report (2.12.14)

0 files   -   3  0 suites   - 3   0s ⏱️ -23s
0 tests  - 23  0 ✔️  - 23  0 💤 ±0  0 ❌ ±0 

Results for commit f93f0ae. ± Comparison against base commit 174ed63.

This pull request removes 23 tests.
splain.ErrorsCompat2_13_1Spec ‑ ambiguous implicits
splain.ErrorsSpec ‑ Tuple1
splain.ErrorsSpec ‑ aux type
splain.ErrorsSpec ‑ byname higher order
splain.ErrorsSpec ‑ compact tree printing
splain.ErrorsSpec ‑ deep hole
splain.ErrorsSpec ‑ disambiguate types
splain.ErrorsSpec ‑ found/required type diff
splain.ErrorsSpec ‑ implicit resolution chains
splain.ErrorsSpec ‑ linebreak long infix types
…

@github-actions
Copy link

Test Report (2.12.13)

0 files   -   3  0 suites   - 3   0s ⏱️ -27s
0 tests  - 23  0 ✔️  - 23  0 💤 ±0  0 ❌ ±0 

Results for commit f93f0ae. ± Comparison against base commit 174ed63.

This pull request removes 23 tests.
splain.ErrorsCompat2_13_1Spec ‑ ambiguous implicits
splain.ErrorsSpec ‑ Tuple1
splain.ErrorsSpec ‑ aux type
splain.ErrorsSpec ‑ byname higher order
splain.ErrorsSpec ‑ compact tree printing
splain.ErrorsSpec ‑ deep hole
splain.ErrorsSpec ‑ disambiguate types
splain.ErrorsSpec ‑ found/required type diff
splain.ErrorsSpec ‑ implicit resolution chains
splain.ErrorsSpec ‑ linebreak long infix types
…

@tek
Copy link
Owner Author

tek commented Jun 21, 2021

also, needs a bit of refactoring for the compatibilty mess

@github-actions
Copy link

Test Report (2.13.2)

0 files   -   4  0 suites   - 4   0s ⏱️ -25s
0 tests  - 24  0 ✔️  - 24  0 💤 ±0  0 ❌ ±0 

Results for commit f93f0ae. ± Comparison against base commit 174ed63.

This pull request removes 24 tests.
splain.ErrorsCompat2_13_2Spec ‑ ambiguous implicits
splain.ErrorsCompatSpec ‑ byname
splain.ErrorsSpec ‑ Tuple1
splain.ErrorsSpec ‑ aux type
splain.ErrorsSpec ‑ byname higher order
splain.ErrorsSpec ‑ compact tree printing
splain.ErrorsSpec ‑ deep hole
splain.ErrorsSpec ‑ disambiguate types
splain.ErrorsSpec ‑ found/required type diff
splain.ErrorsSpec ‑ implicit resolution chains
…

@tribbloid
Copy link
Collaborator

wow you are fast, I'll try to learn what it takes to keep up with the compiler

@github-actions
Copy link

Test Report (2.13.1)

0 files   -   4  0 suites   - 4   0s ⏱️ -21s
0 tests  - 24  0 ✔️  - 24  0 💤 ±0  0 ❌ ±0 

Results for commit f93f0ae. ± Comparison against base commit 174ed63.

This pull request removes 24 tests.
splain.ErrorsCompat2_13_1Spec ‑ ambiguous implicits
splain.ErrorsCompatSpec ‑ byname
splain.ErrorsSpec ‑ Tuple1
splain.ErrorsSpec ‑ aux type
splain.ErrorsSpec ‑ byname higher order
splain.ErrorsSpec ‑ compact tree printing
splain.ErrorsSpec ‑ deep hole
splain.ErrorsSpec ‑ disambiguate types
splain.ErrorsSpec ‑ found/required type diff
splain.ErrorsSpec ‑ implicit resolution chains
…

@github-actions
Copy link

Test Report (2.13.3)

0 files   -   4  0 suites   - 4   0s ⏱️ -24s
0 tests  - 24  0 ✔️  - 24  0 💤 ±0  0 ❌ ±0 

Results for commit f93f0ae. ± Comparison against base commit 174ed63.

This pull request removes 24 tests.
splain.ErrorsCompat2_13_2Spec ‑ ambiguous implicits
splain.ErrorsCompatSpec ‑ byname
splain.ErrorsSpec ‑ Tuple1
splain.ErrorsSpec ‑ aux type
splain.ErrorsSpec ‑ byname higher order
splain.ErrorsSpec ‑ compact tree printing
splain.ErrorsSpec ‑ deep hole
splain.ErrorsSpec ‑ disambiguate types
splain.ErrorsSpec ‑ found/required type diff
splain.ErrorsSpec ‑ implicit resolution chains
…

@github-actions
Copy link

Test Report (2.13.4)

0 files   -   4  0 suites   - 4   0s ⏱️ -26s
0 tests  - 24  0 ✔️  - 24  0 💤 ±0  0 ❌ ±0 

Results for commit f93f0ae. ± Comparison against base commit 174ed63.

This pull request removes 24 tests.
splain.ErrorsCompat2_13_2Spec ‑ ambiguous implicits
splain.ErrorsCompatSpec ‑ byname
splain.ErrorsSpec ‑ Tuple1
splain.ErrorsSpec ‑ aux type
splain.ErrorsSpec ‑ byname higher order
splain.ErrorsSpec ‑ compact tree printing
splain.ErrorsSpec ‑ deep hole
splain.ErrorsSpec ‑ disambiguate types
splain.ErrorsSpec ‑ found/required type diff
splain.ErrorsSpec ‑ implicit resolution chains
…

@github-actions
Copy link

Test Report (2.13.5)

0 files   -   4  0 suites   - 4   0s ⏱️ -25s
0 tests  - 24  0 ✔️  - 24  0 💤 ±0  0 ❌ ±0 

Results for commit f93f0ae. ± Comparison against base commit 174ed63.

This pull request removes 24 tests.
splain.ErrorsCompat2_13_2Spec ‑ ambiguous implicits
splain.ErrorsCompatSpec ‑ byname
splain.ErrorsSpec ‑ Tuple1
splain.ErrorsSpec ‑ aux type
splain.ErrorsSpec ‑ byname higher order
splain.ErrorsSpec ‑ compact tree printing
splain.ErrorsSpec ‑ deep hole
splain.ErrorsSpec ‑ disambiguate types
splain.ErrorsSpec ‑ found/required type diff
splain.ErrorsSpec ‑ implicit resolution chains
…

@tek
Copy link
Owner Author

tek commented Jun 21, 2021

well it's not much effort to add the analyzer plugin 😅

@tribbloid
Copy link
Collaborator

I see what you are doing here. The analyser cannot be defined for the old version - because it has to extend a new Scala class. The old code has to extend other old Scala classes that are no longer safe to extend (with constructor hijacking particularly)

In addition, the name scala_2.13.6+ I used is no longer accurate (we have exclusive code for 2.13.6 now)

All these things put together probably means it is hard to catch the deadline. Would you like me to backport the patch for 2.12.14 compatibility to the previous version and call it a release?

@tek
Copy link
Owner Author

tek commented Jun 23, 2021

sounds good, yeah. the whole version specific thing is getting real messy.

Maybe it would be sensible to drop all but the latest versions of 12/13 into a legacy branch and add new features only to 2.12.14 and the analyzer plugin, then release the same code for compat for all the older versions or something…

@tribbloid
Copy link
Collaborator

or we could just announce that "the next version won't have 2.13.6 support, please wait for a hotfix"

@tek
Copy link
Owner Author

tek commented Jun 24, 2021

that's no good, because people have potentially tens of projects at work with different scala versions, and if they have splain in the global sbt config they'd have to screw around with conditionals

@tribbloid
Copy link
Collaborator

done, https://github.com/tek/splain/tree/0.5.8_2.12.14

this should be consistent with latest behaviour in Scala 2.13.6

@tek
Copy link
Owner Author

tek commented Jun 25, 2021

ok, I made releases for both 2.12.14 and 2.13.6 (with an empty analyzer plugin)

@tribbloid
Copy link
Collaborator

Can relax a bit :)

@tribbloid
Copy link
Collaborator

Would you like to talk about roadmap now? If you think that it will be too hard or time consuming to maintain 2 versions (splain and scalac), you can simply declare that 0.5.8 is the last release, and all upcoming patches should directly goes to scala compiler

@tribbloid
Copy link
Collaborator

WTF? When the code was copied to scalac, all automated tests are omitted.

Not cool man, it loses its most valuable asset

@tek
Copy link
Owner Author

tek commented Sep 29, 2021

what was copied to scalac?

regarding roadmap – if we strip down to just the features that aren't in scala, it shouldn't be that much effort. assuming we can get this to be ergonomic

@tribbloid
Copy link
Collaborator

tribbloid commented Sep 29, 2021

what was copied to scalac?

All the production code in scala compiler project, in the following package:

package scala.tools.nsc
package typechecker
package splain

They are copied from this repo. Yet all the unit tests are omitted which makes it impossible to maintain. So obviously you can choose to delete all the code that are already copied, but not before some of us submitting a patch that include all the missing tests (and latest patch on splain 0.5.9). Otherwise, the component will quickly be destroyed by junior PhD students :)

If you want my suggestion, I would recommend setting the current version DIRECTLY TO 1.0-SNAPSHOT, and declare it NOT backward compatible with scala 2.13.5-:

  • If the code is in production, it shouldn't be in alpha or beta (denoted by 0.x.x)
  • There is no need to introduce even more complicate cross-compilation

@tek
Copy link
Owner Author

tek commented Sep 29, 2021

Not sure this is what you mean, but the unit tests are now partests, for example: https://github.com/scala/scala/blob/2.13.x/test/files/run/splain.scala

Regarding your suggestion with the version – sounds good, please go ahead!

@tribbloid
Copy link
Collaborator

  • alright, it still strikes me as undisciplined, a project of this size should always put implementation and test code in the same package.

  • let me edit directly on you branch for the first PR

@tribbloid
Copy link
Collaborator

Did you hardcode all the configurations?

  val opts: mutable.Map[String, String] = mutable.Map(
    keyAll -> "true",
    keyImplicits -> "true",
    keyFoundReq -> "true",
    keyInfix -> "true",
    keyBounds -> "false",
    keyColor -> "true",
    keyBreakInfix -> "0",
    keyCompact -> "false",
    keyTree -> "true",
    keyBoundsImplicits -> "true",
    keyTruncRefined -> "0",
    keyRewrite -> "",
    keyKeepModules -> "0",
  )

Was it because they are already in scalac and you haven't figure out a way to read them?

@tribbloid
Copy link
Collaborator

In scalac it is also hardcoded to 70.

Did you make some political rivals in scala team?

@tek
Copy link
Owner Author

tek commented Sep 30, 2021

Did you hardcode all the configurations?

what do you mean by "hardcode"? those are the defaults that have always been like this

and I'm not sure why the 70 was chosen there…

@tribbloid
Copy link
Collaborator

tribbloid commented Sep 30, 2021

I'm talking about this part:

  val keyAll = "all"
  val keyImplicits = "implicits"
  val keyFoundReq = "foundreq"
  val keyInfix = "infix"
  val keyBounds = "bounds"
  val keyColor = "color"
  val keyBreakInfix = "breakinfix"
  val keyCompact = "compact"
  val keyTree = "tree"
  val keyBoundsImplicits = "boundsimplicits"
  val keyTruncRefined = "truncrefined"
  val keyRewrite = "rewrite"
  val keyKeepModules = "keepmodules"

  val opts: mutable.Map[String, String] = mutable.Map(
    keyAll -> "true",
    keyImplicits -> "true",
    keyFoundReq -> "true",
    keyInfix -> "true",
    keyBounds -> "false",
    keyColor -> "true",
    keyBreakInfix -> "0",
    keyCompact -> "false",
    keyTree -> "true",
    keyBoundsImplicits -> "true",
    keyTruncRefined -> "0",
    keyRewrite -> "",
    keyKeepModules -> "0"
  )

  def opt(key: String, default: String) = opts.getOrElse(key, default)

as you can see, the opts cannot be changed and wasn't read from any external option

@tribbloid
Copy link
Collaborator

Just for curiosity: do you prefer VSCode over IntelliJ IDEA?

Asking because IntelliJ IDEA is mostly a Java IDE, so its indexing is optimised for FileName==ClassName convention which is totally optional in Scala

I'm thinking of breaking one big file into several smaller one that complies with this convention, but only if you feel comfortable with it.

@tribbloid
Copy link
Collaborator

tribbloid commented Sep 30, 2021

Also, this is where 70 came from:

  val breakInfixLength: Int = 70

In SplainFormatting, scalac.

I've already complained on their feature forum:

https://contributors.scala-lang.org/t/need-to-backport-all-the-missing-splain-configurations-for-vimplicits-vtype-diffs-options/5274

We should move discussion there to engage more people.

def invalid(opt: String) = error(s"splain: invalid option `$opt`")
def setopt(key: String, value: String) =
if (opts.contains(key))
opts.update(key, value)
Copy link
Owner Author

Choose a reason for hiding this comment

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

the options are updated here

@tek
Copy link
Owner Author

tek commented Oct 2, 2021

Just for curiosity: do you prefer VSCode over IntelliJ IDEA?

Asking because IntelliJ IDEA is mostly a Java IDE, so its indexing is optimised for FileName==ClassName convention which is totally optional in Scala

I'm thinking of breaking one big file into several smaller one that complies with this convention, but only if you feel comfortable with it.

I only use neovim, and I'd be fine with breaking files up

@tek
Copy link
Owner Author

tek commented Oct 2, 2021

Also, this is where 70 came from:

I meant that I don't remember why it was set to 70 in the PR

@tribbloid
Copy link
Collaborator

OK this PR should already be included in #66, closing

@tribbloid tribbloid closed this Oct 17, 2021
@tek tek deleted the analyzer-plugin branch October 17, 2021 20:30
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.

None yet

2 participants