Re: [Codenarc-user] Running rules which require compilerPhase > Phases.CONVERSION using Gradle's CodeNarc plugin

[Marcin E. <mar...@pr...>]

So I had a look and this is my current thinking:

1. Add a new *classpath* nested element and *classpathref* property to
CodeNarc's ant task. They would be analogous to the ones with the same
names defined for groovyc ant task (
http://groovy-lang.org/groovyc.html#_ant_task) and thus use a way of
defining classpaths ant users are familiar with (by being backed with
org.apache.tools.ant.types.Path).
2. Use the newly added classpath definition to create a new *UrlClassLoader*
with current context classloader as it's parent in *CodeNarcTask.execute()*.
Set the newly created classloader as the context classloader around the
call to *CodeNarcRunner.execute()*.
3. Get a new version of CodeNarc released so that the new functionality can
be integrated with Gradle's CodeNarc plugin.
4. Add a new *FileCollection compilationClasspath* property to Gradle's
CodeNarc task that will be used to configure *classpath* nested element of
*CodeNarcTask* in *CodeNarcInvoker*. If the *compilationClasspath* property
is not set then *classpath* nested element is not configured allowing for
backwards compatibility with older CodeNarc versions.

Another option would be to change which classloader is used when CodeNarc
compiles analysed code to the one that was used to load its classes, but
I'd rather not do that because:
- it's a potentially breaking change
- you should not be forced to use the same classpath for both the tool
(Codenarc) and the classes you compile

Chris, I would highly appreciate if you could confirm that you would be ok
with such change and would be happy to do a release after it's merged in to
unlock integrating the change with Gradle. Upon receiving a go ahead from
you I will send a message to Gradle dev list asking if they are ok with my
proposed change in point 4.

Cheers,
Marcin

On Wed, Mar 8, 2017 at 7:56 AM, Marcin Erdmann <mar...@pr...>
wrote:

> Thanks for your response, Chris.
>
> It's good to understand that using the context classloader when compiling
> analysed code is not a conscious design decision but just the way it works
> at the moment.
>
> I will do some more digging around on Friday which is my OSS day at work.
> I will check what the context classloader is set to when the extended rules
> are tested within CodeNarc's test suite and investigate if specifying a
> classpath to be used for the context classloader as part of CodeNarc's ant
> task is an option and how we would deal with that backwards incompatible
> change in Gradle's CodeNarc plugin.
>
> Cheers,
> Marcin
>
> On Wed, 8 Mar 2017 at 02:32, Chris Mair <cm...@gm...> wrote:
>
>> Ugh. Sorry for the delayed response. I wasn't sure how to respond, and I
>> was hoping that maybe someone else might have some useful input.
>>
>> I am not very familiar with the internals of the Gradle CodeNarc plugin,
>> so I'm not sure what to suggest from that side.
>>
>> I am not philosophically opposed to changing the CodeNarc classloader,
>> but I am nervous about unintended consequences. Support for the "enhanced"
>> rules was a later addition, and presumably less used, though I'm still
>> surprised no one else has complained about this.
>>
>> On Fri, Mar 3, 2017 at 8:39 AM, Marcin Erdmann <mar...@pr...
>> > wrote:
>>
>> Hi,
>>
>> I wrote a custom rule which requires SEMANTIC_ANALYSIS compiler phase and
>> I'm trying to run it using Gradle. Unfortunately I'm getting compilation
>> errors upon analysis telling me that compiler cannot load any of the
>> classes imported by the analysed code, even though I checked and they are
>> available in the codenarc configuration of my project which is added by
>> Gradle's CodeNarc plugin and used to load CodeNarc classes, see:
>>
>> - https://github.com/gradle/gradle/blob/v3.3.0/
>> subprojects/code-quality/src/main/groovy/org/gradle/api/plugins/quality/
>> CodeNarcPlugin.java#L72
>> - https://github.com/gradle/gradle/blob/v3.3.0/
>> subprojects/code-quality/src/main/groovy/org/gradle/api/plugins/quality/
>> CodeNarcPlugin.java#L89
>> - https://github.com/gradle/gradle/blob/v3.3.0/
>> subprojects/code-quality/src/main/groovy/org/gradle/api/
>> plugins/quality/CodeNarc.java#L104
>> - https://github.com/gradle/gradle/blob/v3.3.0/
>> subprojects/code-quality/src/main/groovy/org/gradle/api/
>> plugins/quality/internal/CodeNarcInvoker.groovy#L45
>>
>> I then dug around and realised that when CodeNarc compiles analysed code
>> it uses the context class loader, see:
>> - https://github.com/CodeNarc/CodeNarc/blob/v0.26.0/src/
>> main/groovy/org/codenarc/source/AbstractSourceCode.groovy#L88
>> - https://github.com/groovy/groovy-core/blob/GROOVY_2_1_8/
>> src/main/org/codehaus/groovy/control/CompilationUnit.java#L94
>> https://github.com/groovy/groovy-core/blob/GROOVY_2_1_8/
>> src/main/org/codehaus/groovy/control/CompilationUnit.java#L136
>> - https://github.com/groovy/groovy-core/blob/GROOVY_2_1_8/
>> src/main/org/codehaus/groovy/control/ProcessingUnit.java#L64
>> - https://github.com/groovy/groovy-core/blob/GROOVY_2_1_8/
>> src/main/org/codehaus/groovy/control/ProcessingUnit.java#L103
>>
>> And I finally realised that Gradle's BasicAntBuilder replaces the context
>> class loader to only contain Ant classes which means that not even Groovy
>> classes are available on the compilation classpath when CodeNarc compiles
>> the analysed code:
>> - https://github.com/gradle/gradle/blob/v3.3.0/
>> subprojects/code-quality/src/main/groovy/org/gradle/api/
>> plugins/quality/internal/CodeNarcInvoker.groovy#L45
>> - https://github.com/gradle/gradle/blob/v3.3.0/subprojects/core/src/main/
>> java/org/gradle/api/internal/project/antbuilder/
>> DefaultIsolatedAntBuilder.java#L151
>> - https://github.com/gradle/gradle/blob/v3.3.0/subprojects/core/src/main/
>> java/org/gradle/api/internal/project/ant/BasicAntBuilder.java#L76
>>
>> Given the above, here are my questions:
>>
>> 1. Has anybody ever managed to get their projects set up correctly and
>> get violations for one of the built-in CodeNarc Enhanced Classpath Rules,
>> e.g. CloneWithoutCloneable? If that's the case can they please explain how
>> they achieved it? If one is to believe comments in Gradle's codebase (e.g.
>> https://github.com/gradle/gradle/blob/v3.3.0/subprojects/core/src/main/
>> java/org/gradle/api/internal/project/antbuilder/
>> DefaultIsolatedAntBuilder.java#L140) then it looks like a similar
>> problem to the one occurring when using Gradle would occur when using Ant.
>>
>> 2. Is there a reason for CodeNarc to use the context class loader instead
>> of the class loader that loaded CodeNarc when compiling the analysed code?
>> Given that it is a breaking change, would a PR changing that class loader
>> to the one which loaded CodeNarc be a valid option? It is a breaking change
>> but I don't see a reason why somebody would depend on current behaviour as
>> it actually prevents usage of Enhanced Classpath Rules. Or should I look
>> into changing Gradle's CodeNarc plugin to set the context class loader as
>> per CodeNarc's expectations?
>>
>> Cheers,
>> Marcin
>>
>>
>>
>> ------------------------------------------------------------
>> ------------------
>> Check out the vibrant tech community on one of the world's most
>> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
>> _______________________________________________
>> Codenarc-user mailing list
>> Cod...@li...
>> https://lists.sourceforge.net/lists/listinfo/codenarc-user
>>
>>
>>