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.

Sign up for GitHub

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

Jump to bottom

bump compiler-interface and capture the diagnosticCode #15565

Merged
merged 1 commit into from
Aug 1, 2022
Merged

bump compiler-interface and capture the diagnosticCode #15565

merged 1 commit into from
Aug 1, 2022

Conversation

ckipp01
Copy link
Member

@ckipp01 ckipp01 commented Jul 1, 2022

NOTE: It's important that in Problem we don't construct the actual
DiagnosticCode unless the method is called to get it. By doing this we
keep compatibility ensuring that older build tools can still use the
bridge just fine. That's what the sbt-test/sbt-bridge tests ensuring
that this is still usable by an older tool using the old Problem. This
then unlocks forwarding the diagnostic code on for tools to use.

refs: #14904

@ckipp01 ckipp01 mentioned this pull request Jul 1, 2022
Open
16 tasks
ckipp01 added a commit to ckipp01/mill that referenced this pull request Jul 1, 2022
@ckipp01
Bump zinc and account for diagnostic code
4063921 
At the moment this is just testing a bit. I see you already have a test
PR to bump zinc in com-lihaoyi#1845 but one
of the things that this brings in is the changes to `Problem` I made in
sbt/sbt#6874 that expose the diagnostic code of
the diagnostic coming from dotc. I have been doing some work on that on
the compiler side in scala/scala3#15565 and
wanted to try it out with Mill.

I tried to mimic the way you currently have it set up, so let me know if
it's not the direction you'd want to go. However, the idea here would be
that the diagnostic code is forwarded when diagnostics are published via
BSP so that Metals could then capture that code and know what code
actions to offer. You can see more of the big picture in scala/scala3#14904.
@ckipp01 ckipp01 mentioned this pull request Jul 1, 2022
Merged
@ckipp01 ckipp01 marked this pull request as draft July 1, 2022 17:01
@ckipp01 ckipp01 mentioned this pull request Jul 2, 2022
Merged
ckipp01 added a commit to ckipp01/mill that referenced this pull request Jul 22, 2022
@ckipp01
Bump zinc and account for diagnostic code
0ab83de 
At the moment this is just testing a bit. I see you already have a test
PR to bump zinc in com-lihaoyi#1845 but one
of the things that this brings in is the changes to `Problem` I made in
sbt/sbt#6874 that expose the diagnostic code of
the diagnostic coming from dotc. I have been doing some work on that on
the compiler side in scala/scala3#15565 and
wanted to try it out with Mill.

I tried to mimic the way you currently have it set up, so let me know if
it's not the direction you'd want to go. However, the idea here would be
that the diagnostic code is forwarded when diagnostics are published via
BSP so that Metals could then capture that code and know what code
actions to offer. You can see more of the big picture in scala/scala3#14904.
ckipp01 added a commit to ckipp01/dotty that referenced this pull request Jul 22, 2022
@ckipp01
dep: bump sbt to 1.7.1
8ccd9ab 
Looking at the
[commit](scala@fa063b6)
that added 1.6.2 I believe I have everything updated that needs to be. I
am updating this because it's needed for a test that I'm adding in scala#15565,
but thought it'd be best to do this bump in a separate pr.
@ckipp01 ckipp01 mentioned this pull request Jul 22, 2022
Merged
ckipp01 added a commit to ckipp01/dotty that referenced this pull request Jul 22, 2022
@ckipp01
dep: bump sbt to 1.7.1
1e1ccd7 
Looking at the
[commit](scala@fa063b6)
that added 1.6.2 I believe I have everything updated that needs to be. I
am updating this because it's needed for a test that I'm adding in scala#15565,
but thought it'd be best to do this bump in a separate pr.
ckipp01 added a commit to ckipp01/dotty that referenced this pull request Jul 26, 2022
@ckipp01
dep: bump sbt to 1.7.1
de97d4b 
Looking at the
[commit](scala@fa063b6)
that added 1.6.2 I believe I have everything updated that needs to be. I
am updating this because it's needed for a test that I'm adding in scala#15565,
but thought it'd be best to do this bump in a separate pr.
@ckipp01
bump compiler-interface and capture the diagnosticCode
aabe2aa 
NOTE: It's important that in `Problem` we don't construct the actual
`DiagnosticCode` _unless_ the method is called to get it. By doing this we
keep compatibility ensuring that older build tools can still use the
bridge just fine. That's what the `sbt-test/sbt-bridge` tests ensuring
that this is still usable by an older tool using the old `Problem`. This
then unlocks forwarding the diagnostic code on for tools to use.

refs: scala#14904
@ckipp01 ckipp01 marked this pull request as ready for review July 28, 2022 08:11
@dwijnand dwijnand added the needs-minor-release This PR cannot be merged until the next minor release label Jul 28, 2022
@ckipp01
Copy link
Member Author

ckipp01 commented Aug 1, 2022

Btw just to make sure I understand the label @dwijnand. So this can't get merged until 3.2.x is released?

@dwijnand
Copy link
Member

dwijnand commented Aug 1, 2022

I'll bring this up at the meeting (cc @Kordyjan). Generally I would prefer for such a change not to ship in a patch release. So, either we end up needing an RC4 for 3.2.0, in which case I would look to landing this in main and backporting it to https://github.com/lampepfl/dotty/commits/release-3.2.0 so it can be part of 3.2.0-RC4+. Or we wait keep it unmerged until main is targeting for 3.3.0 (as opposed to the 3.2.x releases we'll be doing). Unless we decide let's just ship this in a random patch release.

@dwijnand dwijnand added this to the 3.2.0 backports milestone Aug 1, 2022
@dwijnand dwijnand removed the needs-minor-release This PR cannot be merged until the next minor release label Aug 1, 2022
@dwijnand dwijnand merged commit 718576c into scala:main Aug 1, 2022
@dwijnand dwijnand mentioned this pull request Aug 1, 2022
Merged
@dwijnand dwijnand removed this from the 3.2.0 backports milestone Aug 1, 2022
@dwijnand dwijnand added the backport:done This PR was successfully backported. label Aug 1, 2022
@ckipp01 ckipp01 deleted the bridgeChanges branch August 1, 2022 15:49
ckipp01 added a commit to ckipp01/mill that referenced this pull request Aug 7, 2022
@ckipp01
Bump zinc and account for diagnostic code
8520913 
At the moment this is just testing a bit. I see you already have a test
PR to bump zinc in com-lihaoyi#1845 but one
of the things that this brings in is the changes to `Problem` I made in
sbt/sbt#6874 that expose the diagnostic code of
the diagnostic coming from dotc. I have been doing some work on that on
the compiler side in scala/scala3#15565 and
wanted to try it out with Mill.

I tried to mimic the way you currently have it set up, so let me know if
it's not the direction you'd want to go. However, the idea here would be
that the diagnostic code is forwarded when diagnostics are published via
BSP so that Metals could then capture that code and know what code
actions to offer. You can see more of the big picture in scala/scala3#14904.
ckipp01 added a commit to ckipp01/sbt that referenced this pull request Aug 9, 2022
@ckipp01 @adpi2
feat: start forwarding diagnosticCode via BSP
15a45cb 
This change is a continuation of the work that was done in
sbt#6874 to allow the Scala 3 compiler to
forward the `diagnosticCode` of an error as well as the other normal
things. This change in dotty was merged in
scala/scala3#15565 and also backported so it
will be in the 3.2.x series release. This change captures the
`diagnosticCode` and forwards it on via BSP so that tools like Metals
can can use this code.

You can see this in the BSP trace now for a diagnostic:

For example with some code that contains the following:

```scala
val x: Int = "hi"
```

You'll see the code in the BSP diagnostic:
```  "diagnostics": [
    {
      "range": {
        "start": {
          "line": 9,
          "character": 15
        },
        "end": {
          "line": 9,
          "character": 19
        }
      },
      "severity": 1,
      "code": "7",
      "source": "sbt",
      "message": "Found:    (\u001b[32m\"hi\"\u001b[0m : String)\nRequired: Int\n\nThe following import might make progress towards fixing the problem:\n\n  import sourcecode.Text.generate\n\n"
    }
  ]
```

Co-authored-by: Adrien Piquerez <[email protected]>
Refs: scala/scala3#14904
@ckipp01 ckipp01 mentioned this pull request Aug 9, 2022
Merged
ckipp01 added a commit to ckipp01/metals that referenced this pull request Aug 9, 2022
@ckipp01
feat: capture and forward diagnosticCode
2e707fc 
This relates to the grand plan of
scala/scala3#14904 and recently forwarding
the `diagnosticCode` has been merged in
scala/scala3#15565 and also backported so it
should show up in the 3.2.x series. While this pr isn't super exciting,
it's just making sure we capture the code and forward it, this should
unlock _much_ better ways to determine what code actions are available
for a given diagnostic. Meaning we don't have to do lovely things like
regex on the diagnostic message for Scala 3 diagnostics.

NOTE: that this does need some more changes in the build servers before
this is usable. So we can wait for those to be merged in if you'd like.

- [ ] sbt - sbt/sbt#6998
- [ ] Bloop - scalacenter/bloop#1750
- [ ] Mill - com-lihaoyi/mill#1912

Now if you look at the trace file for a diagnostic you'll see the
addition of the code:

```
  "diagnostics": [
    {
      "range": {
        "start": {
          "line": 9,
          "character": 15
        },
        "end": {
          "line": 9,
          "character": 19
        }
      },
      "severity": 1,
      "code": "7",
      "source": "sbt",
      "message": "Found:    (\u001b[32m\"hi\"\u001b[0m : String)\nRequired: Int\n\nThe following import might make progress towards fixing the problem:\n\n  import sourcecode.Text.generate\n\n"
    }
  ],
```

Refs: scala/scala3#14904
@ckipp01 ckipp01 mentioned this pull request Aug 9, 2022
Merged
3 tasks
eed3si9n pushed a commit to eed3si9n/sbt that referenced this pull request Aug 10, 2022
@ckipp01 @adpi2 @eed3si9n
feat: start forwarding diagnosticCode via BSP
92a9f0e 
This change is a continuation of the work that was done in
sbt#6874 to allow the Scala 3 compiler to
forward the `diagnosticCode` of an error as well as the other normal
things. This change in dotty was merged in
scala/scala3#15565 and also backported so it
will be in the 3.2.x series release. This change captures the
`diagnosticCode` and forwards it on via BSP so that tools like Metals
can can use this code.

You can see this in the BSP trace now for a diagnostic:

For example with some code that contains the following:

```scala
val x: Int = "hi"
```

You'll see the code in the BSP diagnostic:
```  "diagnostics": [
    {
      "range": {
        "start": {
          "line": 9,
          "character": 15
        },
        "end": {
          "line": 9,
          "character": 19
        }
      },
      "severity": 1,
      "code": "7",
      "source": "sbt",
      "message": "Found:    (\u001b[32m\"hi\"\u001b[0m : String)\nRequired: Int\n\nThe following import might make progress towards fixing the problem:\n\n  import sourcecode.Text.generate\n\n"
    }
  ]
```

Co-authored-by: Adrien Piquerez <[email protected]>
Refs: scala/scala3#14904
@eed3si9n eed3si9n mentioned this pull request Aug 10, 2022
Merged
eed3si9n pushed a commit to eed3si9n/sbt that referenced this pull request Oct 2, 2022
@ckipp01 @adpi2 @eed3si9n
feat: start forwarding diagnosticCode via BSP
046abaa 
This change is a continuation of the work that was done in
sbt#6874 to allow the Scala 3 compiler to
forward the `diagnosticCode` of an error as well as the other normal
things. This change in dotty was merged in
scala/scala3#15565 and also backported so it
will be in the 3.2.x series release. This change captures the
`diagnosticCode` and forwards it on via BSP so that tools like Metals
can can use this code.

You can see this in the BSP trace now for a diagnostic:

For example with some code that contains the following:

```scala
val x: Int = "hi"
```

You'll see the code in the BSP diagnostic:
```  "diagnostics": [
    {
      "range": {
        "start": {
          "line": 9,
          "character": 15
        },
        "end": {
          "line": 9,
          "character": 19
        }
      },
      "severity": 1,
      "code": "7",
      "source": "sbt",
      "message": "Found:    (\u001b[32m\"hi\"\u001b[0m : String)\nRequired: Int\n\nThe following import might make progress towards fixing the problem:\n\n  import sourcecode.Text.generate\n\n"
    }
  ]
```

Co-authored-by: Adrien Piquerez <[email protected]>
Refs: scala/scala3#14904
@eed3si9n eed3si9n mentioned this pull request Oct 2, 2022
Merged
ckipp01 added a commit to ckipp01/mill that referenced this pull request Oct 3, 2022
@ckipp01
Bump zinc and account for diagnostic code
84f81a8 
At the moment this is just testing a bit. I see you already have a test
PR to bump zinc in com-lihaoyi#1845 but one
of the things that this brings in is the changes to `Problem` I made in
sbt/sbt#6874 that expose the diagnostic code of
the diagnostic coming from dotc. I have been doing some work on that on
the compiler side in scala/scala3#15565 and
wanted to try it out with Mill.

I tried to mimic the way you currently have it set up, so let me know if
it's not the direction you'd want to go. However, the idea here would be
that the diagnostic code is forwarded when diagnostics are published via
BSP so that Metals could then capture that code and know what code
actions to offer. You can see more of the big picture in scala/scala3#14904.
ckipp01 added a commit to ckipp01/mill that referenced this pull request Oct 8, 2022
@ckipp01
Bump zinc and account for diagnostic code
08e9b7f 
At the moment this is just testing a bit. I see you already have a test
PR to bump zinc in com-lihaoyi#1845 but one
of the things that this brings in is the changes to `Problem` I made in
sbt/sbt#6874 that expose the diagnostic code of
the diagnostic coming from dotc. I have been doing some work on that on
the compiler side in scala/scala3#15565 and
wanted to try it out with Mill.

I tried to mimic the way you currently have it set up, so let me know if
it's not the direction you'd want to go. However, the idea here would be
that the diagnostic code is forwarded when diagnostics are published via
BSP so that Metals could then capture that code and know what code
actions to offer. You can see more of the big picture in scala/scala3#14904.
lefou pushed a commit to com-lihaoyi/mill that referenced this pull request Oct 8, 2022
@ckipp01
Bump zinc and account for diagnostic code (#1912)
0ec0e9e 
Newer zinc brings in is the changes to `Problem` I made in
sbt/sbt#6874 that expose the diagnostic code of
the diagnostic coming from dotc. I have been doing some work on that on
the compiler side in scala/scala3#15565 and
wanted to try it out with Mill.

I tried to mimic the way you currently have it set up, so let me know if
it's not the direction you'd want to go. However, the idea here would be
that the diagnostic code is forwarded when diagnostics are published via
BSP so that Metals could then capture that code and know what code
actions to offer. You can see more of the big picture in
scala/scala3#14904.

Pull request: #1912
ckipp01 added a commit to ckipp01/metals that referenced this pull request Oct 24, 2022
@ckipp01
feat: capture and forward diagnosticCode
980d444 
This relates to the grand plan of
scala/scala3#14904 and recently forwarding
the `diagnosticCode` has been merged in
scala/scala3#15565 and also backported so it
should show up in the 3.2.x series. While this pr isn't super exciting,
it's just making sure we capture the code and forward it, this should
unlock _much_ better ways to determine what code actions are available
for a given diagnostic. Meaning we don't have to do lovely things like
regex on the diagnostic message for Scala 3 diagnostics.

NOTE: that this does need some more changes in the build servers before
this is usable. So we can wait for those to be merged in if you'd like.

- [ ] sbt - sbt/sbt#6998
- [ ] Bloop - scalacenter/bloop#1750
- [ ] Mill - com-lihaoyi/mill#1912

Now if you look at the trace file for a diagnostic you'll see the
addition of the code:

```
  "diagnostics": [
    {
      "range": {
        "start": {
          "line": 9,
          "character": 15
        },
        "end": {
          "line": 9,
          "character": 19
        }
      },
      "severity": 1,
      "code": "7",
      "source": "sbt",
      "message": "Found:    (\u001b[32m\"hi\"\u001b[0m : String)\nRequired: Int\n\nThe following import might make progress towards fixing the problem:\n\n  import sourcecode.Text.generate\n\n"
    }
  ],
```

Refs: scala/scala3#14904
tgodzik pushed a commit to scalameta/metals that referenced this pull request Oct 25, 2022
@ckipp01 @tgodzik
feat: capture and forward diagnosticCode
6fca4bc 
This relates to the grand plan of
scala/scala3#14904 and recently forwarding
the `diagnosticCode` has been merged in
scala/scala3#15565 and also backported so it
should show up in the 3.2.x series. While this pr isn't super exciting,
it's just making sure we capture the code and forward it, this should
unlock _much_ better ways to determine what code actions are available
for a given diagnostic. Meaning we don't have to do lovely things like
regex on the diagnostic message for Scala 3 diagnostics.

NOTE: that this does need some more changes in the build servers before
this is usable. So we can wait for those to be merged in if you'd like.

- [ ] sbt - sbt/sbt#6998
- [ ] Bloop - scalacenter/bloop#1750
- [ ] Mill - com-lihaoyi/mill#1912

Now if you look at the trace file for a diagnostic you'll see the
addition of the code:

```
  "diagnostics": [
    {
      "range": {
        "start": {
          "line": 9,
          "character": 15
        },
        "end": {
          "line": 9,
          "character": 19
        }
      },
      "severity": 1,
      "code": "7",
      "source": "sbt",
      "message": "Found:    (\u001b[32m\"hi\"\u001b[0m : String)\nRequired: Int\n\nThe following import might make progress towards fixing the problem:\n\n  import sourcecode.Text.generate\n\n"
    }
  ],
```

Refs: scala/scala3#14904
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dwijnand dwijnand dwijnand approved these changes

Assignees
No one assigned
Labels
backport:done This PR was successfully backported.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ckipp01 @dwijnand