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

Remove unnecessary, and incorrect, Java error message translation #524

Merged
merged 1 commit into from Apr 5, 2018

Conversation

Projects
None yet
3 participants
@retronym
Copy link
Member

commented Apr 5, 2018

Given:

class C {
    class D {}
    void test() {
        D.this.toString();
    }
}

Before:

⚡ sbt compile
[info] Loading settings from dirtymoney.sbt,gpg.sbt,idea.sbt ...
[info] Loading global plugins from /Users/jz/.sbt/1.0/plugins
[info] Loading project definition from /private/tmp/javac-diagnostic/project
[info] Set current project to javac-diagnostic (in build file:/private/tmp/javac-diagnostic/)
[info] Executing in batch mode. For better performance use sbt's shell
[info] Compiling 1 Java source to /private/tmp/javac-diagnostic/target/scala-2.12/classes ...
[error] /private/tmp/javac-diagnostic/src/main/java/example/Test.java:4:1:  C.D
[error]         D.this.toString();
[error] (Compile / compileIncremental) javac returned non-zero exit code
[error] Total time: 0 s, completed 05/04/2018 12:52:16 PM
/tmp/javac-diagnostic

After:

⚡ sbt -sbt-version 1.1.3-pre-98d08039e
Getting org.scala-sbt sbt 1.1.3-pre-98d08039e  (this may take some time)...
:: retrieving :: org.scala-sbt#boot-app
    confs: [default]
    77 artifacts copied, 0 already retrieved (27803kB/251ms)
...
[info] Set current project to javac-diagnostic (in build file:/private/tmp/javac-diagnostic/)
[info] sbt server started at local:///Users/jz/.sbt/1.0/server/3cc671e1c4b514e4d201/sock
sbt:javac-diagnostic> compile
[info] Compiling 1 Java source to /private/tmp/javac-diagnostic/target/scala-2.12/classes ...
[error] /private/tmp/javac-diagnostic/src/main/java/example/Test.java:4:1: not an enclosing class: C.D
[error]         D.this.toString();
[error] (Compile / compileIncremental) javac returned non-zero exit code
[error] Total time: 0 s, completed 05/04/2018 1:02:53 PM
sbt:javac-diagnostic>

Fixes #523

Remove unnecessary, and incorrect, Java error message translation
Given:

```java
class C {
    class D {}
    void test() {
        D.this.toString();
    }
}
```

Before:

```
⚡ sbt compile
[info] Loading settings from dirtymoney.sbt,gpg.sbt,idea.sbt ...
[info] Loading global plugins from /Users/jz/.sbt/1.0/plugins
[info] Loading project definition from /private/tmp/javac-diagnostic/project
[info] Set current project to javac-diagnostic (in build file:/private/tmp/javac-diagnostic/)
[info] Executing in batch mode. For better performance use sbt's shell
[info] Compiling 1 Java source to /private/tmp/javac-diagnostic/target/scala-2.12/classes ...
[error] /private/tmp/javac-diagnostic/src/main/java/example/Test.java:4:1:  C.D
[error]         D.this.toString();
[error] (Compile / compileIncremental) javac returned non-zero exit code
[error] Total time: 0 s, completed 05/04/2018 12:52:16 PM
/tmp/javac-diagnostic
```

After:

```
⚡ sbt -sbt-version 1.1.3-pre-98d08039e
Getting org.scala-sbt sbt 1.1.3-pre-98d08039e  (this may take some time)...
:: retrieving :: org.scala-sbt#boot-app
    confs: [default]
    77 artifacts copied, 0 already retrieved (27803kB/251ms)
...
[info] Set current project to javac-diagnostic (in build file:/private/tmp/javac-diagnostic/)
[info] sbt server started at local:///Users/jz/.sbt/1.0/server/3cc671e1c4b514e4d201/sock
sbt:javac-diagnostic> compile
[info] Compiling 1 Java source to /private/tmp/javac-diagnostic/target/scala-2.12/classes ...
[error] /private/tmp/javac-diagnostic/src/main/java/example/Test.java:4:1: not an enclosing class: C.D
[error]         D.this.toString();
[error] (Compile / compileIncremental) javac returned non-zero exit code
[error] Total time: 0 s, completed 05/04/2018 1:02:53 PM
sbt:javac-diagnostic>
```

@retronym retronym requested a review from jvican Apr 5, 2018

@jvican

jvican approved these changes Apr 5, 2018

@dwijnand

This comment has been minimized.

Copy link
Member

commented Apr 5, 2018

looks like something that could have a test for it.

or is there an impediment to a test for this case?

@dwijnand

This comment has been minimized.

Copy link
Member

commented Apr 5, 2018

btw looking in sbt/sbt#1702 I can't find any more info about fixedDiagnosticMessage.

@retronym

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2018

I wasn't sure how to structure the a (scripted?) test to make assertions about the javac output. I'm sure there is a way, but I'm trying to get out of my SBT fix 🐰 hole for the week, so was a bit lazy.

@retronym

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2018

My best guess is that the code used to call toString (which does include the file/line number), but was changed to use getMessage before sbt/sbt#1702 landed.

@dwijnand

This comment has been minimized.

Copy link
Member

commented Apr 5, 2018

so was a bit lazy

no worries, I'll try and follow this up with a test.

@dwijnand dwijnand added this to the 1.1.4 milestone Apr 5, 2018

@dwijnand dwijnand merged commit 9a62679 into sbt:1.1.x Apr 5, 2018

1 check passed

continuous-integration/drone/pr the build was successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.