Re: IDEA plugin for error-prone

796 views
Skip to first unread message

Alex Eagle

unread,
Mar 5, 2014, 4:52:54 PM3/5/14
to Nikolay Chashnikov, error-pro...@googlegroups.com
Thanks Nikolay, I'm excited to have your help keeping our integration working.

From a quick read, it sounds like your hack is similar to the way we wire up error-prone in our ErrorProneCompiler main, which is also a bit of a hack, since javac doesn't really expose a plugin API.
Maybe Eddie or Liam will have a chance to look at this sooner than I, so forwarding to our discuss list.

-Alex


On Wed, Mar 5, 2014 at 4:53 AM, Nikolay Chashnikov <Nikolay.C...@jetbrains.com> wrote:
Hello, Alex,

I've extracted API which allows plugins to use custom compilers instead of Javac in the external build process. Since the option to switch off 'External build' mode was removed in IDEA 13.0 it doesn't make sense to keep the old implementation for 'in-process build', so I've created a new plugin from scratch which integrates error-prone compiler with the external build using the newly created API. The plugin sources will be published to https://github.com/JetBrains/intellij-plugins soon. Also I'll upload the plugin to the plugin repository after the next IDEA 13.1 EAP build which will contain the necessary API.

There was one technical problem and I'm not sure that I'd solved it properly. The external build system runs java compiler in-process and deeply integrates with javac API (in particular it provides its own implementation of javax.tools.StandardJavaFileManager). So we cannot use ErrorProneCompiler class directly. Currently it's implemented in the following way: it creates the default javac instance using ToolProvider.getSystemJavaCompiler(), calls getTask() method and then puts ErrorProneScanner to com.sun.tools.javac.util.Context instance obtained from the task, calls ErrorReportingJavaCompiler.preRegister method and then invokes the task. It seems to work but looks like a hack. It would be much simpler if error-prone library provides an implementation of javax.tools.JavaCompiler.


Hey guys,
I've finally released the 1.0 of our plugin to the repository.
http://plugins.jetbrains.com/plugin/7349

I've had a few people say that they use the "Make project automatically" setting so losing the "Use external build" doesn't work for them.
What's the next step to get the compiler to be able to use our wrapper class around javac.Main ?

And it would be fine to plan a move of the plugin code to your repository, if you care to maintain it.

(Also I'm sorry about Dmitriy leaving but I'm excited he's come to Google! :)


On Wed, Jul 24, 2013 at 3:02 AM, Sergey Simonchik <sergey.simonchik@jetbrains.com <mailto:sergey.simonchik@jetbrains.com>> wrote:

    Hi,

    Here is a version of the plugin that is compatible with IDEA 12.1.4:
    https://code.google.com/r/sergeysimonchik-error-prone/source/detail?r=2b6519cd67eef6c9046d6106eadb19227a237e16

    This version has one downside: it works if "Use external build" isn't selected (Settings | Compiler | Use external build).

    To make the plugin working with external build, please contact Nikolay Chashnikov <Nikolay.Chashnikov@jetbrains.com>
    <mailto:Nikolay.Chashnikov@jetbrains.com> as one of designers of our compiler model.


    Sergey



    On 07/20/2013 02:26 PM, Sergey Simonchik wrote:
    Here it is: Maxim.M...@jetbrains.com <mailto:Maxim.Mossienko@jetbrains.com>


    About making the plugin compatible with IDEA 12.1.4:
    It seems our external compiler concept has been changed a bit. Some more time is needed to fix that. I'll report back as it's done.

    On 07/19/2013 02:00 AM, Alex Eagle wrote:
    Okay, should I get in touch with Maxim then? What's his email?


    On Mon, Jul 15, 2013 at 1:18 AM, Sergey Simonchik <sergey.simonchik@jetbrains.com <mailto:sergey.simonchik@jetbrains.com>> wrote:

        Hey Alex!

        Sorry for the delay.
        I'll look into it soon and report back.

        About moving the plugin: Maxim is in charge here as he is a project manager of IDEA (and I'm WebStorm developer).

        Sergey


        On 07/12/2013 11:59 PM, Alex Eagle wrote:
        Hey again,
        We have some IJ12 users who are trying the plugin, but it hasn't been updated in a year:
        https://code.google.com/p/error-prone/issues/detail?id=159

        We're getting close to cutting our 1.0 release, so hopefully we can move the plugin to your repo?


        On Wed, Jun 13, 2012 at 7:32 AM, Sergey Simonchik <sergey.simonchik@jetbrains.com <mailto:sergey.simonchik@jetbrains.com>> wrote:

            Yes, error-prone.googlecode.com <http://error-prone.googlecode.com> is a right repository.

            Yes, Apache license is ok.
            Yes, we're ok without any copyright header.
            :)


            On 06/09/2012 09:28 PM, Alex Eagle wrote:
            Awesome! Thanks for putting that together. I gave it a spin... at first I was missing the com.intellij.compiler APIs. I added
            idea.jar to my IDEA SDK classpath, which fixed that. Helps to have some experience with intellij plugins!

            Then it ran fine. I tested by changing my compiler to "javac with error-prone" and compiled my test class. The compiler errors look
            good, and navigate me to the right spot in the source. So I think this seems quite ready to ship as a first version. I posted to G+
            because I was very excited.

            Which repository should we commit this to? I'm guessing since this is still early experimental, we could check it into
            error-prone.googlecode.com <http://error-prone.googlecode.com>? I know that google legal is concerned about us contributing to your

            repo under the Czech republic laws, for whatever reason. If you're ok with Apache license that's easier. I'm not so sure about what
            copyright header to claim. I suppose since you're the author we can do without any copyright header? (I'm really glad I'm an engineer
            and not a lawyer :)

            -Alex

            On Sat, Jun 9, 2012 at 8:09 AM, Sergey Simonchik <sergey.simonchik@jetbrains.com <mailto:sergey.simonchik@jetbrains.com>> wrote:

                Hi Alex,

                Sorry for the delay.
                See attached source code of the plugin.
                It's mostly a copy-paste from com.intellij.compiler.impl.javaCompiler.javac.JavacCompiler.



                On 06/09/2012 04:48 AM, Alex Eagle wrote:
                Ping!


                On Mon, May 28, 2012 at 11:24 AM, Alex Eagle <alex...@google.com <mailto:alex...@google.com>> wrote:

                    It's not enough to change the command line parameters to Javac. We have to subclass com.sun.tools.javac.main.Main to "plug
                    in" to the compiler, so you have to change one line in the in-process javac strategy, where you choose the entry point class.
                    That's why I think it will need a plugin, though a very simple one.

                    For example, here's how the ant task is implemented:
                    http://code.google.com/p/error-prone/source/browse/ant/src/main/java/com/google/errorprone/ErrorProneAntCompilerAdapter.java

                    Right now, we provide all our output "UI" via javac's existing error reporting, so it's formatted exactly the same as a
                    regular compile error. We summarize the suggested fix there as well.

                    It would be awesome to provide the suggested fix data in a structured form so IDEA could actually perform the change on the
                    PSI. But I think we should leave that for a second iteration.

                    -Alex

                    On Mon, May 28, 2012 at 2:16 AM, Sergey Simonchik <sergey.simonchik@jetbrains.com <mailto:sergey.simonchik@jetbrains.com>>

                    wrote:

                        Hi guys,

                        Well, to run javac with error-prone inside IntelliJ IDEA you can tweak "Additional command line parameters" parameter in
                        Settings | Compiler | Java Compiler.

                        To have suggested fixes and refactoring please provide an error message structure description (quick version of how to
                        make javac run with error-prone would also be helpful).

                        Sergey


                        On 05/24/2012 01:08 PM, Sergey Simonchik wrote:

                            Hey Alex,

                            Definitely it's an interesting project.
                            I've shared the info with our team.
                            We'll response in a day or two.

                            --
                            Sergey

                            On 05/23/2012 08:57 PM, Alex Eagle wrote:

                                Hey Sergey,

                                Thanks for all the help with JSTestDriver, and also for discussing Kotlin with me and Kevin Bourillion a few
                                months ago.

                                I've got another project cooking, called error-prone. After working with Findbugs guy Bill Pugh for a couple
                                years, we decided to re-implement some of the 'always-wrong' findbugs detectors directly in Javac as compile
                                errors, which makes it universally toolable and gets the feedback a lot sooner. See
                                http://code.google.com/p/error-prone/wiki/ComparisonWithFindbugs

                                We're hoping to release in the next couple weeks. I started work on a compiler plugin for IDEA, but it will take
                                me some more time. I realized that since you guys are all super hackers, you could probably do it in a couple
                                hours, so I thought I'd check in and see if you know anyone on your team who might want to do it.

                                If so, I'll give you the quick version of how to make javac run with error-prone. It's only a couple lines,
                                barring any classloader issues.

                                Thanks!
                                -Alex













!DSPAM:38,5291169c255131380419189!


--
Nikolay Chashnikov
JetBrains
http://www.jetbrains.com
"Develop with pleasure!"

Nikolay Chashnikov

unread,
Mar 6, 2014, 9:47:53 AM3/6/14
to Alex Eagle, error-pro...@googlegroups.com
In fact I looked at ErrorProneCompiler and related classes to repeat the way error-prone integrates into javac. However I think it would be better to
keep these hacks inside your repository to ensure that they won't be broken as your code changes. I've extracted this code into a separate class
implementing javax.tools.JavaCompiler
(https://github.com/JetBrains/intellij-plugins/blob/master/error-prone/jps-plugin/src/org/intellij/errorProne/ErrorProneJavaCompiler.java), what do
you think about moving this class to your repository (and get rid of reflection calls in this class)?

> Thanks Nikolay, I'm excited to have your help keeping our integration working.
>
> From a quick read, it sounds like your hack is similar to the way we wire up error-prone in our ErrorProneCompiler main, which is also a bit of a
> hack, since javac doesn't really expose a plugin API.
> Maybe Eddie or Liam will have a chance to look at this sooner than I, so forwarding to our discuss list.
>
> -Alex
>
>
> On Wed, Mar 5, 2014 at 4:53 AM, Nikolay Chashnikov <Nikolay.C...@jetbrains.com <mailto:Nikolay.C...@jetbrains.com>> wrote:
>
> Hello, Alex,
>
> I've extracted API which allows plugins to use custom compilers instead of Javac in the external build process. Since the option to switch off
> 'External build' mode was removed in IDEA 13.0 it doesn't make sense to keep the old implementation for 'in-process build', so I've created a new
> plugin from scratch which integrates error-prone compiler with the external build using the newly created API. The plugin sources will be
> published to https://github.com/JetBrains/__intellij-plugins <https://github.com/JetBrains/intellij-plugins> soon. Also I'll upload the plugin to
> the plugin repository after the next IDEA 13.1 EAP build which will contain the necessary API.
>
> There was one technical problem and I'm not sure that I'd solved it properly. The external build system runs java compiler in-process and deeply
> integrates with javac API (in particular it provides its own implementation of javax.tools.__StandardJavaFileManager). So we cannot use
> ErrorProneCompiler class directly. Currently it's implemented in the following way: it creates the default javac instance using
> ToolProvider.__getSystemJavaCompiler(), calls getTask() method and then puts ErrorProneScanner to com.sun.tools.javac.util.__Context instance
> obtained from the task, calls ErrorReportingJavaCompiler.__preRegister method and then invokes the task. It seems to work but looks like a hack.
> It would be much simpler if error-prone library provides an implementation of javax.tools.JavaCompiler.
>
>
> Hey guys,
> I've finally released the 1.0 of our plugin to the repository.
> http://plugins.jetbrains.com/__plugin/7349 <http://plugins.jetbrains.com/plugin/7349>
>
> I've had a few people say that they use the "Make project automatically" setting so losing the "Use external build" doesn't work for them.
> What's the next step to get the compiler to be able to use our wrapper class around javac.Main ?
>
> And it would be fine to plan a move of the plugin code to your repository, if you care to maintain it.
>
> (Also I'm sorry about Dmitriy leaving but I'm excited he's come to Google! :)
>
>
> On Wed, Jul 24, 2013 at 3:02 AM, Sergey Simonchik <sergey.simonchik@jetbrains.__com <mailto:sergey.s...@jetbrains.com>
> <mailto:sergey.simonchik@__jetbrains.com <mailto:sergey.s...@jetbrains.com>>> wrote:
>
> Hi,
>
> Here is a version of the plugin that is compatible with IDEA 12.1.4:
> https://code.google.com/r/__sergeysimonchik-error-prone/__source/detail?r=__2b6519cd67eef6c9046d6106eadb19__227a237e16
> <https://code.google.com/r/sergeysimonchik-error-prone/source/detail?r=2b6519cd67eef6c9046d6106eadb19227a237e16>
>
> This version has one downside: it works if "Use external build" isn't selected (Settings | Compiler | Use external build).
>
> To make the plugin working with external build, please contact Nikolay Chashnikov <Nikolay.Chashnikov@jetbrains.__com
> <mailto:Nikolay.C...@jetbrains.com>>
> <mailto:Nikolay.Chashnikov@__jetbrains.com <mailto:Nikolay.C...@jetbrains.com>> as one of designers of our compiler model.
>
>
> Sergey
>
>
>
> On 07/20/2013 02:26 PM, Sergey Simonchik wrote:
>
> Here it is: Maxim.M...@jetbrains.com <mailto:Maxim.M...@jetbrains.com> <mailto:Maxim.Mossienko@__jetbrains.com
> <mailto:Maxim.M...@jetbrains.com>>
>
>
> About making the plugin compatible with IDEA 12.1.4:
> It seems our external compiler concept has been changed a bit. Some more time is needed to fix that. I'll report back as it's done.
>
> On 07/19/2013 02:00 AM, Alex Eagle wrote:
>
> Okay, should I get in touch with Maxim then? What's his email?
>
>
> On Mon, Jul 15, 2013 at 1:18 AM, Sergey Simonchik <sergey.simonchik@jetbrains.__com <mailto:sergey.s...@jetbrains.com>
> <mailto:sergey.simonchik@__jetbrains.com <mailto:sergey.s...@jetbrains.com>>> wrote:
>
> Hey Alex!
>
> Sorry for the delay.
> I'll look into it soon and report back.
>
> About moving the plugin: Maxim is in charge here as he is a project manager of IDEA (and I'm WebStorm developer).
>
> Sergey
>
>
> On 07/12/2013 11:59 PM, Alex Eagle wrote:
>
> Hey again,
> We have some IJ12 users who are trying the plugin, but it hasn't been updated in a year:
> https://code.google.com/p/__error-prone/issues/detail?id=__159 <https://code.google.com/p/error-prone/issues/detail?id=159>
>
> We're getting close to cutting our 1.0 release, so hopefully we can move the plugin to your repo?
>
>
> On Wed, Jun 13, 2012 at 7:32 AM, Sergey Simonchik <sergey.simonchik@jetbrains.__com
> <mailto:sergey.s...@jetbrains.com> <mailto:sergey.simonchik@__jetbrains.com <mailto:sergey.s...@jetbrains.com>>> wrote:
>
> Yes, error-prone.googlecode.com <http://error-prone.googlecode.com> <http://error-prone.__googlecode.com
> <http://error-prone.googlecode.com>> is a right repository.
>
> Yes, Apache license is ok.
> Yes, we're ok without any copyright header.
> :)
>
>
> On 06/09/2012 09:28 PM, Alex Eagle wrote:
>
> Awesome! Thanks for putting that together. I gave it a spin... at first I was missing the com.intellij.compiler
> APIs. I added
> idea.jar to my IDEA SDK classpath, which fixed that. Helps to have some experience with intellij plugins!
>
> Then it ran fine. I tested by changing my compiler to "javac with error-prone" and compiled my test class. The
> compiler errors look
> good, and navigate me to the right spot in the source. So I think this seems quite ready to ship as a first
> version. I posted to G+
> because I was very excited.
>
> Which repository should we commit this to? I'm guessing since this is still early experimental, we could check it
> into
> error-prone.googlecode.com <http://error-prone.googlecode.com> <http://error-prone.__googlecode.com
> <http://error-prone.googlecode.com>>? I know that google legal is concerned about us contributing to your
>
> repo under the Czech republic laws, for whatever reason. If you're ok with Apache license that's easier. I'm not
> so sure about what
> copyright header to claim. I suppose since you're the author we can do without any copyright header? (I'm really
> glad I'm an engineer
> and not a lawyer :)
>
> -Alex
>
> On Sat, Jun 9, 2012 at 8:09 AM, Sergey Simonchik <sergey.simonchik@jetbrains.__com
> <mailto:sergey.s...@jetbrains.com> <mailto:sergey.simonchik@__jetbrains.com <mailto:sergey.s...@jetbrains.com>>> wrote:
>
> Hi Alex,
>
> Sorry for the delay.
> See attached source code of the plugin.
> It's mostly a copy-paste from com.intellij.compiler.impl.__javaCompiler.javac.__JavacCompiler.
>
>
>
> On 06/09/2012 04:48 AM, Alex Eagle wrote:
>
> Ping!
>
>
> On Mon, May 28, 2012 at 11:24 AM, Alex Eagle <alex...@google.com <mailto:alex...@google.com>
> <mailto:alex...@google.com <mailto:alex...@google.com>>> wrote:
>
> It's not enough to change the command line parameters to Javac. We have to subclass
> com.sun.tools.javac.main.Main to "plug
> in" to the compiler, so you have to change one line in the in-process javac strategy, where you
> choose the entry point class.
> That's why I think it will need a plugin, though a very simple one.
>
> For example, here's how the ant task is implemented:
> http://code.google.com/p/__error-prone/source/browse/ant/__src/main/java/com/google/__errorprone/__ErrorProneAntCompilerAdapter.__java
> <http://code.google.com/p/error-prone/source/browse/ant/src/main/java/com/google/errorprone/ErrorProneAntCompilerAdapter.java>
>
> Right now, we provide all our output "UI" via javac's existing error reporting, so it's formatted
> exactly the same as a
> regular compile error. We summarize the suggested fix there as well.
>
> It would be awesome to provide the suggested fix data in a structured form so IDEA could actually
> perform the change on the
> PSI. But I think we should leave that for a second iteration.
>
> -Alex
>
> On Mon, May 28, 2012 at 2:16 AM, Sergey Simonchik <sergey.simonchik@jetbrains.__com
> <mailto:sergey.s...@jetbrains.com> <mailto:sergey.simonchik@__jetbrains.com <mailto:sergey.s...@jetbrains.com>>>
> http://code.google.com/p/__error-prone/wiki/__ComparisonWithFindbugs
> <http://code.google.com/p/error-prone/wiki/ComparisonWithFindbugs>
>
> We're hoping to release in the next couple weeks. I started work on a compiler plugin for
> IDEA, but it will take
> me some more time. I realized that since you guys are all super hackers, you could
> probably do it in a couple
> hours, so I thought I'd check in and see if you know anyone on your team who might want
> to do it.
>
> If so, I'll give you the quick version of how to make javac run with error-prone. It's
> only a couple lines,
> barring any classloader issues.
>
> Thanks!
> -Alex
>
>
>
>
>
>
>
>
>
>
>
>
>
> u>5291169c255131380419189!
>
>
>
> --
> Nikolay Chashnikov
> JetBrains
> http://www.jetbrains.com
> "Develop with pleasure!"
>
>
> !DSPAM:38,53179cd5257035786344116!

Alex Eagle

unread,
Mar 6, 2014, 1:00:18 PM3/6/14
to Nikolay Chashnikov, error-pro...@googlegroups.com
Sounds like a good approach to me. I created a bug in our issue tracker so we don't lose this.


        On Wed, Jul 24, 2013 at 3:02 AM, Sergey Simonchik <sergey.simonchik@jetbrains.__com <mailto:sergey.simonchik@jetbrains.com>

        <mailto:sergey.simonchik@__jetbrains.com <mailto:sergey.simonchik@jetbrains.com>>> wrote:

             Hi,

             Here is a version of the plugin that is compatible with IDEA 12.1.4:
        https://code.google.com/r/__sergeysimonchik-error-prone/__source/detail?r=__2b6519cd67eef6c9046d6106eadb19__227a237e16

        <https://code.google.com/r/sergeysimonchik-error-prone/source/detail?r=2b6519cd67eef6c9046d6106eadb19227a237e16>

             This version has one downside: it works if "Use external build" isn't selected (Settings | Compiler | Use external build).

             To make the plugin working with external build, please contact Nikolay Chashnikov <Nikolay.Chashnikov@jetbrains.__com
        <mailto:Nikolay.Chashnikov@jetbrains.com>>
             <mailto:Nikolay.Chashnikov@__jetbrains.com <mailto:Nikolay.Chashnikov@jetbrains.com>> as one of designers of our compiler model.



             Sergey



             On 07/20/2013 02:26 PM, Sergey Simonchik wrote:

                 Here it is: Maxim.M...@jetbrains.com <mailto:Maxim.Mossienko@jetbrains.com> <mailto:Maxim.Mossienko@__jetbrains.com

            <mailto:Maxim.Mossienko@jetbrains.com>>



                 About making the plugin compatible with IDEA 12.1.4:
                 It seems our external compiler concept has been changed a bit. Some more time is needed to fix that. I'll report back as it's done.

                 On 07/19/2013 02:00 AM, Alex Eagle wrote:

                     Okay, should I get in touch with Maxim then? What's his email?


                     On Mon, Jul 15, 2013 at 1:18 AM, Sergey Simonchik <sergey.simonchik@jetbrains.__com <mailto:sergey.simonchik@jetbrains.com>

                <mailto:sergey.simonchik@__jetbrains.com <mailto:sergey.simonchik@jetbrains.com>>> wrote:

                         Hey Alex!

                         Sorry for the delay.
                         I'll look into it soon and report back.

                         About moving the plugin: Maxim is in charge here as he is a project manager of IDEA (and I'm WebStorm developer).

                         Sergey


                         On 07/12/2013 11:59 PM, Alex Eagle wrote:

                             Hey again,
                             We have some IJ12 users who are trying the plugin, but it hasn't been updated in a year:
                    https://code.google.com/p/__error-prone/issues/detail?id=__159 <https://code.google.com/p/error-prone/issues/detail?id=159>


                             We're getting close to cutting our 1.0 release, so hopefully we can move the plugin to your repo?


                             On Wed, Jun 13, 2012 at 7:32 AM, Sergey Simonchik <sergey.simonchik@jetbrains.__com
                            <mailto:sergey.simonchik@jetbrains.com> <mailto:sergey.simonchik@__jetbrains.com <mailto:sergey.simonchik@jetbrains.com>>>
Message has been deleted

Alex Eagle

unread,
Mar 25, 2014, 3:11:10 PM3/25/14
to error-pro...@googlegroups.com, Nikolay Chashnikov
FYI the associated bug is marked fixed now.


On Thursday, March 6, 2014 10:00:18 AM UTC-8, Alex Eagle wrote:
Sounds like a good approach to me. I created a bug in our issue tracker so we don't lose this.

Nikolay Chashnikov

unread,
Mar 27, 2014, 8:13:33 AM3/27/14
to Alex Eagle, error-pro...@googlegroups.com
Hello,
I've uploaded new version of the plugin to http://plugins.jetbrains.com/plugin/7349.
Currently it includes version 1.1.1 of error-prone-core library. When you release a new version containing implementation of JavaCompiler I'll update the plugin to use it and remove my hacks.
Thank you for your co-operation!

Jens

unread,
Mar 27, 2014, 9:21:26 AM3/27/14
to error-pro...@googlegroups.com, Alex Eagle, nikolay.c...@jetbrains.com
I've uploaded new version of the plugin to http://plugins.jetbrains.com/plugin/7349.
Currently it includes version 1.1.1 of error-prone-core library. When you release a new version containing implementation of JavaCompiler I'll update the plugin to use it and remove my hacks.
Thank you for your co-operation!

Compilation failed with 6 errors :) Seems to work well using IntelliJ 13.1.1 on Mac OS.

Now it would be nice to make error-prone more configurable (custom bug patterns, change of maturity level)

-- J.
Reply all
Reply to author
Forward
0 new messages