Marcin,
Sounds good. Perhaps you can provide an *ignoreInnerClasses *config
property on the rule.
After I replied before, I realized that providing a Set of annotation names
that were required should probably be qualified/named to be clear that it
is "one of these" and not "all of these" that are required. So please
consider naming that config property to make that clear.
Thanks.
Chris
On Sun, Mar 5, 2017 at 2:03 PM, Marcin Erdmann <marcin.erdmann@...>
wrote:
> Chris,
>
> RequiredClassAnnotation sounds like a very good idea. It would cover my
> use case assuming that it would pass if one annotation from a set of
> configured annotations is present and could be configured to ignore inner
> classes. Doing it in this more generic way would also open up opportunities
> for others to use it for something other than ensuring compilation mode is
> explicitly specified. So it looks like it's a double win that way - I'll
> implement it in the generic way you described.
>
> Cheers,
> Marcin
>
> On Sat, Mar 4, 2017 at 2:15 AM, Chris Mair <cmair42@...> wrote:
>
>> Marcin,
>>
>> As one of our star contributors, you get extra influence!
>>
>> The *MissingOverrideAnnotation* rule sounds good in any case. I assume
>> you can put that in the "enhanced" ruleset, for now at least.
>>
>> I don't have a big problem excluding a rule from the CodeNarc internal
>> ruleset; I think we already do that in a few cases. So, that is not a
>> concern.
>>
>> The *CompilationModeNotSpecified* rule, as described, sounds a bit more
>> custom and specialized. I'd be more hesitant to pull that one in, unless we
>> can make a case for wider adoption/usage.
>>
>> I have been considering implementing generic *RequiredClassAnnotation*
>> rule and/or *IllegalClassAnnotation* rules (and possibly also *Required/Illegal
>> Method* rules) that check for annotations on classes/methods. Would it
>> be any value to come at it from that generic point of view -- perhaps
>> allowing a Set of annotation names that were required? The idea would then
>> be to use the existing Rule configuration to apply the rule to the desired
>> set of classes.
>>
>> Thanks.
>> Chris
>>
>> On Fri, Mar 3, 2017 at 9:11 AM, Marcin Erdmann <marcin.erdmann@...
>> > wrote:
>>
>>> I'm looking into contributing two rules I'm working on at the moment. I
>>> see them being potentially contentious and would like to ask if they have a
>>> chance of being accepted before putting in the effort to submit a PR. I'm
>>> fairly sure that I will want to reuse them across my projects so I will
>>> most probably open source them stand alone if you don't see them being a
>>> fit for the set of core rules so don't feel bad saying no.
>>>
>>> The rules are:
>>>
>>> 1. MissingOverrideAnnotation - a rule which enforces annotating methods
>>> which override or implement other methods with @Override as suggested in
>>> "Item 36: Consistently use the Override annotation" of Joshua Bloch's
>>> "Effective Java". This rule requires SEMANTIC_ANALYSIS compiler phase.
>>>
>>> 2. CompilationModeNotSpecified - a rule which requires every class
>>> (apart from inner classes, depending on configuration) to be annotated with
>>> either @CompileStatic or @CompileDynamic. The rationale is that we want to
>>> compile statically all of our production classes (dynamic compilation of a
>>> class or method is allowed if explicitly enabled using @CompileDynamic). We
>>> could in theory use a compiler configuration to achieve that instead but
>>> support for these is limited in IntelliJ - configuration is applied to the
>>> whole project which means that we would have problems disabling static
>>> compilation for our tests and we clearly don't want to have different
>>> compilation modes between build and IDE if we were not to apply a compiler
>>> config in the IDE.
>>>
>>> I understand that all of the built-in rules are applied to CodeNarc's
>>> codebase. While the first rule might trigger a lot of violations I can see
>>> it being justified to add all these missing @Override annotations. The
>>> second rule is on the other hand highly opinionated and I don't see value
>>> in applying it to CodeNarc's code base. Would that mean that it's a bad
>>> candidate for the set of built-in rules?
>>>
>>> Thanks,
>>> Marcin
>>>
>>> ------------------------------------------------------------
>>> ------------------
>>> Check out the vibrant tech community on one of the world's most
>>> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
>>> _______________________________________________
>>> Codenarc-developer mailing list
>>> Codenarc-developer@...
>>> https://lists.sourceforge.net/lists/listinfo/codenarc-developer
>>>
>>>
>>
>

Chris,
RequiredClassAnnotation sounds like a very good idea. It would cover my use
case assuming that it would pass if one annotation from a set of configured
annotations is present and could be configured to ignore inner classes.
Doing it in this more generic way would also open up opportunities for
others to use it for something other than ensuring compilation mode is
explicitly specified. So it looks like it's a double win that way - I'll
implement it in the generic way you described.
Cheers,
Marcin
On Sat, Mar 4, 2017 at 2:15 AM, Chris Mair <cmair42@...> wrote:
> Marcin,
>
> As one of our star contributors, you get extra influence!
>
> The *MissingOverrideAnnotation* rule sounds good in any case. I assume
> you can put that in the "enhanced" ruleset, for now at least.
>
> I don't have a big problem excluding a rule from the CodeNarc internal
> ruleset; I think we already do that in a few cases. So, that is not a
> concern.
>
> The *CompilationModeNotSpecified* rule, as described, sounds a bit more
> custom and specialized. I'd be more hesitant to pull that one in, unless we
> can make a case for wider adoption/usage.
>
> I have been considering implementing generic *RequiredClassAnnotation*
> rule and/or *IllegalClassAnnotation* rules (and possibly also *Required/Illegal
> Method* rules) that check for annotations on classes/methods. Would it be
> any value to come at it from that generic point of view -- perhaps allowing
> a Set of annotation names that were required? The idea would then be to use
> the existing Rule configuration to apply the rule to the desired set of
> classes.
>
> Thanks.
> Chris
>
> On Fri, Mar 3, 2017 at 9:11 AM, Marcin Erdmann <marcin.erdmann@...>
> wrote:
>
>> I'm looking into contributing two rules I'm working on at the moment. I
>> see them being potentially contentious and would like to ask if they have a
>> chance of being accepted before putting in the effort to submit a PR. I'm
>> fairly sure that I will want to reuse them across my projects so I will
>> most probably open source them stand alone if you don't see them being a
>> fit for the set of core rules so don't feel bad saying no.
>>
>> The rules are:
>>
>> 1. MissingOverrideAnnotation - a rule which enforces annotating methods
>> which override or implement other methods with @Override as suggested in
>> "Item 36: Consistently use the Override annotation" of Joshua Bloch's
>> "Effective Java". This rule requires SEMANTIC_ANALYSIS compiler phase.
>>
>> 2. CompilationModeNotSpecified - a rule which requires every class (apart
>> from inner classes, depending on configuration) to be annotated with either
>> @CompileStatic or @CompileDynamic. The rationale is that we want to compile
>> statically all of our production classes (dynamic compilation of a class or
>> method is allowed if explicitly enabled using @CompileDynamic). We could in
>> theory use a compiler configuration to achieve that instead but support for
>> these is limited in IntelliJ - configuration is applied to the whole
>> project which means that we would have problems disabling static
>> compilation for our tests and we clearly don't want to have different
>> compilation modes between build and IDE if we were not to apply a compiler
>> config in the IDE.
>>
>> I understand that all of the built-in rules are applied to CodeNarc's
>> codebase. While the first rule might trigger a lot of violations I can see
>> it being justified to add all these missing @Override annotations. The
>> second rule is on the other hand highly opinionated and I don't see value
>> in applying it to CodeNarc's code base. Would that mean that it's a bad
>> candidate for the set of built-in rules?
>>
>> Thanks,
>> Marcin
>>
>> ------------------------------------------------------------
>> ------------------
>> Check out the vibrant tech community on one of the world's most
>> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
>> _______________________________________________
>> Codenarc-developer mailing list
>> Codenarc-developer@...
>> https://lists.sourceforge.net/lists/listinfo/codenarc-developer
>>
>>
>

Marcin,
As one of our star contributors, you get extra influence!
The *MissingOverrideAnnotation* rule sounds good in any case. I assume you
can put that in the "enhanced" ruleset, for now at least.
I don't have a big problem excluding a rule from the CodeNarc internal
ruleset; I think we already do that in a few cases. So, that is not a
concern.
The *CompilationModeNotSpecified* rule, as described, sounds a bit more
custom and specialized. I'd be more hesitant to pull that one in, unless we
can make a case for wider adoption/usage.
I have been considering implementing generic *RequiredClassAnnotation* rule
and/or *IllegalClassAnnotation* rules (and possibly also *Required/Illegal
Method* rules) that check for annotations on classes/methods. Would it be
any value to come at it from that generic point of view -- perhaps allowing
a Set of annotation names that were required? The idea would then be to use
the existing Rule configuration to apply the rule to the desired set of
classes.
Thanks.
Chris
On Fri, Mar 3, 2017 at 9:11 AM, Marcin Erdmann <marcin.erdmann@...>
wrote:
> I'm looking into contributing two rules I'm working on at the moment. I
> see them being potentially contentious and would like to ask if they have a
> chance of being accepted before putting in the effort to submit a PR. I'm
> fairly sure that I will want to reuse them across my projects so I will
> most probably open source them stand alone if you don't see them being a
> fit for the set of core rules so don't feel bad saying no.
>
> The rules are:
>
> 1. MissingOverrideAnnotation - a rule which enforces annotating methods
> which override or implement other methods with @Override as suggested in
> "Item 36: Consistently use the Override annotation" of Joshua Bloch's
> "Effective Java". This rule requires SEMANTIC_ANALYSIS compiler phase.
>
> 2. CompilationModeNotSpecified - a rule which requires every class (apart
> from inner classes, depending on configuration) to be annotated with either
> @CompileStatic or @CompileDynamic. The rationale is that we want to compile
> statically all of our production classes (dynamic compilation of a class or
> method is allowed if explicitly enabled using @CompileDynamic). We could in
> theory use a compiler configuration to achieve that instead but support for
> these is limited in IntelliJ - configuration is applied to the whole
> project which means that we would have problems disabling static
> compilation for our tests and we clearly don't want to have different
> compilation modes between build and IDE if we were not to apply a compiler
> config in the IDE.
>
> I understand that all of the built-in rules are applied to CodeNarc's
> codebase. While the first rule might trigger a lot of violations I can see
> it being justified to add all these missing @Override annotations. The
> second rule is on the other hand highly opinionated and I don't see value
> in applying it to CodeNarc's code base. Would that mean that it's a bad
> candidate for the set of built-in rules?
>
> Thanks,
> Marcin
>
> ------------------------------------------------------------
> ------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> _______________________________________________
> Codenarc-developer mailing list
> Codenarc-developer@...
> https://lists.sourceforge.net/lists/listinfo/codenarc-developer
>
>

I'm looking into contributing two rules I'm working on at the moment. I see
them being potentially contentious and would like to ask if they have a
chance of being accepted before putting in the effort to submit a PR. I'm
fairly sure that I will want to reuse them across my projects so I will
most probably open source them stand alone if you don't see them being a
fit for the set of core rules so don't feel bad saying no.
The rules are:
1. MissingOverrideAnnotation - a rule which enforces annotating methods
which override or implement other methods with @Override as suggested in
"Item 36: Consistently use the Override annotation" of Joshua Bloch's
"Effective Java". This rule requires SEMANTIC_ANALYSIS compiler phase.
2. CompilationModeNotSpecified - a rule which requires every class (apart
from inner classes, depending on configuration) to be annotated with either
@CompileStatic or @CompileDynamic. The rationale is that we want to compile
statically all of our production classes (dynamic compilation of a class or
method is allowed if explicitly enabled using @CompileDynamic). We could in
theory use a compiler configuration to achieve that instead but support for
these is limited in IntelliJ - configuration is applied to the whole
project which means that we would have problems disabling static
compilation for our tests and we clearly don't want to have different
compilation modes between build and IDE if we were not to apply a compiler
config in the IDE.
I understand that all of the built-in rules are applied to CodeNarc's
codebase. While the first rule might trigger a lot of violations I can see
it being justified to add all these missing @Override annotations. The
second rule is on the other hand highly opinionated and I don't see value
in applying it to CodeNarc's code base. Would that mean that it's a bad
candidate for the set of built-in rules?
Thanks,
Marcin

I'm not familiar with Gosu, and unfortunately I also do not use MyEclipse.
Does CodeNarc work with Gosu? What does "codeNarc 1.9 for Gosu" mean? Does
that map to a particular version of CodeNarc (which has a different
versioning scheme: 0.1 .. 0.25).
Chris

CodeNarc is now built using Gradle<http://gradle.org/&gt;, not Maven.
Chris
From: SoftwareEngineering Hauschel [mailto:info@...]
Sent: Friday, January 08, 2016 7:35 AM
To: codenarc-developer@...
Subject: [Codenarc-developer] pom.xml missing
Hi there,
i got the code from https://github.com/CodeNarc/CodeNarc.git, but there is no pom.xml ?!
Is there no more support for maven build?
Fredy

Thanks Brian.
>> I moved it to the Grails package but included it in both Grails and
security rulesets. Is that a valid configuration?
Unfortunately, no. Some of the CodeNarc infrastructure assumes only one rule
per rule name, so it could cause confusion and unexpected results if you
have the same rule in multiple rulesets. I can add a reference to the rule
from the "security" ruleset page.
I am fine with the new name.
I see the pull request. Thanks for the contribution.
Chris
From: Brian Soby [mailto:brian.soby@...]
Sent: Monday, February 03, 2014 2:41 PM
To: Chris Mair
Cc: Artur Gajowy; codenarc-developer@...
Subject: Re: [Codenarc-developer] Mass Assignment rule
Hi Chris,
I updated it based on your suggestions.
Role-based rule groupings seem to make more sense to me than
technology-based ones. For example, I only ever run the security ruleset
since that's my role. I'm less concerned with the technology in which the
issue is found. I can imagine the dev, QA, performance, etc folk would have
similar views. Of course, I do see the need to exclude irrelevant rulesets
that may lead to false positives.
I moved it to the Grails package but included it in both Grails and security
rulesets. Is that a valid configuration?
Also, for naming, it's probably correctly called property binding but the
class of vulnerability is almost exclusively referred to as mass assignment,
mostly due to the GitHub incident. Due to it being the more recognizable
name, would you mind me leaving it as mass assignment?
http://en.wikipedia.org/wiki/Mass_assignment_vulnerabilityhttp://www.infoq.com/news/2012/03/GitHub-Compromised
Thanks
-Brian Soby
On Sun, Feb 2, 2014 at 7:25 AM, Chris Mair <chrismair@...
<mailto:chrismair@...> > wrote:
Brian,
Nice job on the rule. Is that GrailsDomainClass interface still necessary?
If you decide to submit this as a contribution to CodeNarc:
* Include the rule test if you submit a pull request
* I may consider renaming the rule. It misled me at first because I
expected it to be about multiple assignments. I looked at the Grails
documentation <http://grails.org/doc/2.3.x/guide/single.html#dataBinding&gt; ,
and they refer to this as data binding, and specifically as the "mass
property binding mechanism". That is a mouthful, so I'm not sure what the
best rule name would be - GrailsMassPropertyBindingRule?
* I might move this into the Grails package and/or ruleset. I know
it is also appropriate under "security". That is one of my regrets about the
original design of this project - organizing around rulesets instead of
something more flexible like tags.
Thanks.
Chris
From: Brian Soby [mailto:brian.soby@...
<mailto:brian.soby@...> ]
Sent: Thursday, January 30, 2014 4:08 PM
To: Chris Mair
Cc: Artur Gajowy; codenarc-developer@...
<mailto:codenarc-developer@...>
Subject: Re: [Codenarc-developer] (no subject)
As it turns out, CodeNarc can't detect the Grails interfaces so I ended up
reverting to the basic property and variable name checks. It's not ideal,
but I don't see another way at this point. I ran it through our production
code base and it seems solid (no false positives and a good number of hits).
It's possible that's just due to our devs sticking to convention and our
codebase.
I'm not sure what your bar for fragility is but here's the test (caveat: I
started learning groovy 2 weeks ago):
https://github.com/soby/CodeNarc/blob/master/src/main/groovy/org/codenarc/ru
le/security/MassAssignmentRule.groovy
On Wed, Jan 29, 2014 at 4:15 PM, Chris Mair <chrismair@...
<mailto:chrismair@...> > wrote:
Re: running custom rules from the Grails CodeNarc plugin, unfortunately:
http://grails.org/plugin/codenarc
Note that your custom ruleset cannot refer to a custom rule defined within
the Grails application. This is due to a Grails classpath issue, discussed
here
<http://grails.1312388.n4.nabble.com/Can-a-plugin-use-an-application-level-c
lass-td3330014.html> .
It sounds like you could put the rule into another plugin or maybe a jar? I
know, not cool. :(
From: Brian Soby [mailto:brian.soby@...
<mailto:brian.soby@...> ]
Sent: Wednesday, January 29, 2014 6:27 PM
To: Artur Gajowy
Cc: codenarc-developer@...
<mailto:codenarc-developer@...>
Subject: Re: [Codenarc-developer] (no subject)
Hi Artur,
Changing the phase worked with my test case to detect the interface once I
started using AstUtil.classNodeImplementsType for the check. The outstanding
question I had was whether I could skip doing the grails import of
org.codehaus.groovy.grails.commons.GrailsDomainClass by declaring a simple
GrailsDomainClass interface in the rule and checking for that. It looks like
some of the checks done by AstUtil.classNodeImplementsType may match on
simple class name while omitting the package name, which presumably would
work.
The other problem I ran into was how to run my custom CodeNarc rules in the
grails codenarc plugin. Any ideas here? I see comments saying it's not
trivially done.
Thanks
-Brian Soby
On Wed, Jan 29, 2014 at 2:06 PM, Artur Gajowy <artur.gajowy@...
<mailto:artur.gajowy@...> > wrote:
Did you change your rule's compilerPhase to SEMANTIC_ANALYSIS or greater? It
should detect the GrailsDomainClass interface in your test, however I'm not
sure if it would detect the implicit interface implementation done by
Grails.
Be aware of the limitations:
http://codenarc.sourceforge.net/codenarc-enhanced-classpath-rules.htmlhttps://github.com/CodeNarc/CodeNarc/pull/27https://github.com/CodeNarc/CodeNarc/issues/29
If, however, you're ok with running CodeNarc as a test in your project, the
classpath problems won't affect you. On the other hand - CodeNarc users
running it without Grails classes on classpath won't be able to benefit from
the rule.
If you decide to implement the rule - make sure you get the
GrailsDomainClass interface's package right and test it on a real project :)
Best regards,
Artur Gajowy
On 29 January 2014 19:21, Brian Soby <brian.soby@...
<mailto:brian.soby@...> > wrote:
Hi all,
I'm working on a mass assignment rule
(http://sourceforge.net/p/codenarc/feature-requests/392/) and I can't seem
to get type or interface based detection to work. Here's my test code:
public interface GrailsDomainClass {}
class Person implements GrailsDomainClass {
String name
Boolean isAdmin
}
params = [name: 'John', isAdmin: true]
def person = new Person(params)
Ideally, I'd 1) visit the Person constructor 2) Check that it implements
GrailsDomainClass 3) and check that the parameter implements java.util.Map.
Unfortunately, getInterfaces()/getAllInterfaces() seems to always return an
empty set. I've resorted to checking for a variable named "params" but that
seems too fragile. Is there anything I can do to have more robust type and
interface information available?
Thanks
-Brian Soby
----------------------------------------------------------------------------
--
WatchGuard Dimension instantly turns raw network data into actionable
security intelligence. It gives you real-time visual feedback on key
security issues and trends. Skip the complicated setup - simply import
a virtual appliance and go from zero to informed in seconds.
http://pubads.g.doubleclick.net/gampad/clk?id=123612991
<http://pubads.g.doubleclick.net/gampad/clk?id=123612991&iu=/4140/ostg.clktr
k> &iu=/4140/ostg.clktrk
_______________________________________________
Codenarc-developer mailing list
Codenarc-developer@...
<mailto:Codenarc-developer@...>
https://lists.sourceforge.net/lists/listinfo/codenarc-developer
No virus found in this message.
Checked by AVG - http://www.avg.com <http://www.avg.com&gt;
Version: 2014.0.4259 / Virus Database: 3684/7045 - Release Date: 01/30/14
No virus found in this message.
Checked by AVG - http://www.avg.com <http://www.avg.com&gt;
Version: 2014.0.4259 / Virus Database: 3684/7054 - Release Date: 02/02/14

Hi Chris,
I updated it based on your suggestions.
Role-based rule groupings seem to make more sense to me than
technology-based ones. For example, I only ever run the security ruleset
since that's my role. I'm less concerned with the technology in which the
issue is found. I can imagine the dev, QA, performance, etc folk would have
similar views. Of course, I do see the need to exclude irrelevant rulesets
that may lead to false positives.
I moved it to the Grails package but included it in both Grails and
security rulesets. Is that a valid configuration?
Also, for naming, it's probably correctly called property binding but the
class of vulnerability is almost exclusively referred to as mass
assignment, mostly due to the GitHub incident. Due to it being the more
recognizable name, would you mind me leaving it as mass assignment?
http://en.wikipedia.org/wiki/Mass_assignment_vulnerabilityhttp://www.infoq.com/news/2012/03/GitHub-Compromised
Thanks
-Brian Soby
On Sun, Feb 2, 2014 at 7:25 AM, Chris Mair <chrismair@...> wrote:
> Brian,
>
>
>
> Nice job on the rule. Is that *GrailsDomainClass* interface still
> necessary?
>
>
>
> If you decide to submit this as a contribution to *CodeNarc*:
>
> · Include the rule test if you submit a pull request
>
> · I may consider renaming the rule. It misled me at first because
> I expected it to be about multiple assignments. I looked at the Grails
> documentation <http://grails.org/doc/2.3.x/guide/single.html#dataBinding&gt;,
> and they refer to this as *data binding*, and specifically as the "mass
> property binding mechanism". That is a mouthful, so I'm not sure what the
> best rule name would be - *GrailsMassPropertyBindingRule*?
>
> · I might move this into the Grails package and/or ruleset. I
> know it is also appropriate under "security". That is one of my regrets
> about the original design of this project - organizing around *rulesets*instead of something more flexible like
> *tags*.
>
>
>
> Thanks.
>
> Chris
>
>
>
> *From:* Brian Soby [mailto:brian.soby@...]
> *Sent:* Thursday, January 30, 2014 4:08 PM
> *To:* Chris Mair
> *Cc:* Artur Gajowy; codenarc-developer@...
> *Subject:* Re: [Codenarc-developer] (no subject)
>
>
>
> As it turns out, CodeNarc can't detect the Grails interfaces so I ended up
> reverting to the basic property and variable name checks. It's not ideal,
> but I don't see another way at this point. I ran it through our production
> code base and it seems solid (no false positives and a good number of
> hits). It's possible that's just due to our devs sticking to convention and
> our codebase.
>
>
>
> I'm not sure what your bar for fragility is but here's the test (caveat: I
> started learning groovy 2 weeks ago):
>
>
>
>
> https://github.com/soby/CodeNarc/blob/master/src/main/groovy/org/codenarc/rule/security/MassAssignmentRule.groovy
>
>
>
> On Wed, Jan 29, 2014 at 4:15 PM, Chris Mair <chrismair@...>
> wrote:
>
> Re: running custom rules from the Grails *CodeNarc* plugin, unfortunately:
>
>
>
> http://grails.org/plugin/codenarc
>
>
>
> *Note that your custom ruleset cannot refer to a custom rule defined
> within the Grails application. This is due to a Grails classpath issue,
> discussed here
> <http://grails.1312388.n4.nabble.com/Can-a-plugin-use-an-application-level-class-td3330014.html&gt;.*
>
>
>
> It sounds like you could put the rule into another plugin or maybe a jar?
> I know, not cool... L
>
>
>
> *From:* Brian Soby [mailto:brian.soby@...]
> *Sent:* Wednesday, January 29, 2014 6:27 PM
> *To:* Artur Gajowy
> *Cc:* codenarc-developer@...
> *Subject:* Re: [Codenarc-developer] (no subject)
>
>
>
> Hi Artur,
>
>
>
> Changing the phase worked with my test case to detect the interface once I
> started using AstUtil.classNodeImplementsType for the check. The
> outstanding question I had was whether I could skip doing the grails import
> of org.codehaus.groovy.grails.commons.GrailsDomainClass by declaring a
> simple GrailsDomainClass interface in the rule and checking for that. It
> looks like some of the checks done by AstUtil.classNodeImplementsType may
> match on simple class name while omitting the package name, which
> presumably would work.
>
>
>
> The other problem I ran into was how to run my custom CodeNarc rules in
> the grails codenarc plugin. Any ideas here? I see comments saying it's not
> trivially done.
>
>
>
> Thanks
>
> -Brian Soby
>
>
>
> On Wed, Jan 29, 2014 at 2:06 PM, Artur Gajowy <artur.gajowy@...>
> wrote:
>
> Did you change your rule's compilerPhase to SEMANTIC_ANALYSIS or greater?
> It should detect the GrailsDomainClass interface in your test, however I'm
> not sure if it would detect the implicit interface implementation done by
> Grails.
>
>
>
> Be aware of the limitations:
>
> http://codenarc.sourceforge.net/codenarc-enhanced-classpath-rules.html
>
> https://github.com/CodeNarc/CodeNarc/pull/27
>
> https://github.com/CodeNarc/CodeNarc/issues/29
>
>
>
> If, however, you're ok with running CodeNarc as a test in your project,
> the classpath problems won't affect you. On the other hand - CodeNarc users
> running it without Grails classes on classpath won't be able to benefit
> from the rule.
>
>
>
> If you decide to implement the rule - make sure you get the
> GrailsDomainClass interface's package right and test it on a real project :)
>
>
>
> Best regards,
>
> Artur Gajowy
>
>
>
> On 29 January 2014 19:21, Brian Soby <brian.soby@...> wrote:
>
> Hi all,
>
>
>
> I'm working on a mass assignment rule (
> http://sourceforge.net/p/codenarc/feature-requests/392/) and I can't seem
> to get type or interface based detection to work. Here's my test code:
>
>
>
> public interface GrailsDomainClass {}
>
> class Person implements GrailsDomainClass {
>
> String name
>
> Boolean isAdmin
>
> }
>
> params = [name: 'John', isAdmin: true]
>
> def person = new Person(params)
>
>
>
> Ideally, I'd 1) visit the Person constructor 2) Check that it implements
> GrailsDomainClass 3) and check that the parameter implements java.util.Map.
> Unfortunately, getInterfaces()/getAllInterfaces() seems to always return an
> empty set. I've resorted to checking for a variable named "params" but that
> seems too fragile. Is there anything I can do to have more robust type and
> interface information available?
>
>
>
> Thanks
>
> -Brian Soby
>
>
>
>
>
>
>
>
> ------------------------------------------------------------------------------
> WatchGuard Dimension instantly turns raw network data into actionable
> security intelligence. It gives you real-time visual feedback on key
> security issues and trends. Skip the complicated setup - simply import
> a virtual appliance and go from zero to informed in seconds.
>
> http://pubads.g.doubleclick.net/gampad/clk?id=123612991&iu=/4140/ostg.clktrk
> _______________________________________________
> Codenarc-developer mailing list
> Codenarc-developer@...
> https://lists.sourceforge.net/lists/listinfo/codenarc-developer
>
>
>
>
>
>
>
> No virus found in this message.
> Checked by AVG - http://www.avg.com
> Version: 2014.0.4259 / Virus Database: 3684/7045 - Release Date: 01/30/14
>

Brian,
Nice job on the rule. Is that GrailsDomainClass interface still necessary?
If you decide to submit this as a contribution to CodeNarc:
* Include the rule test if you submit a pull request
* I may consider renaming the rule. It misled me at first because I
expected it to be about multiple assignments. I looked at the Grails
documentation <http://grails.org/doc/2.3.x/guide/single.html#dataBinding&gt; ,
and they refer to this as data binding, and specifically as the "mass
property binding mechanism". That is a mouthful, so I'm not sure what the
best rule name would be - GrailsMassPropertyBindingRule?
* I might move this into the Grails package and/or ruleset. I know
it is also appropriate under "security". That is one of my regrets about the
original design of this project - organizing around rulesets instead of
something more flexible like tags.
Thanks.
Chris
From: Brian Soby [mailto:brian.soby@...]
Sent: Thursday, January 30, 2014 4:08 PM
To: Chris Mair
Cc: Artur Gajowy; codenarc-developer@...
Subject: Re: [Codenarc-developer] (no subject)
As it turns out, CodeNarc can't detect the Grails interfaces so I ended up
reverting to the basic property and variable name checks. It's not ideal,
but I don't see another way at this point. I ran it through our production
code base and it seems solid (no false positives and a good number of hits).
It's possible that's just due to our devs sticking to convention and our
codebase.
I'm not sure what your bar for fragility is but here's the test (caveat: I
started learning groovy 2 weeks ago):
https://github.com/soby/CodeNarc/blob/master/src/main/groovy/org/codenarc/ru
le/security/MassAssignmentRule.groovy
On Wed, Jan 29, 2014 at 4:15 PM, Chris Mair <chrismair@...
<mailto:chrismair@...> > wrote:
Re: running custom rules from the Grails CodeNarc plugin, unfortunately:
http://grails.org/plugin/codenarc
Note that your custom ruleset cannot refer to a custom rule defined within
the Grails application. This is due to a Grails classpath issue, discussed
here
<http://grails.1312388.n4.nabble.com/Can-a-plugin-use-an-application-level-c
lass-td3330014.html> .
It sounds like you could put the rule into another plugin or maybe a jar? I
know, not cool. :(
From: Brian Soby [mailto:brian.soby@...
<mailto:brian.soby@...> ]
Sent: Wednesday, January 29, 2014 6:27 PM
To: Artur Gajowy
Cc: codenarc-developer@...
<mailto:codenarc-developer@...>
Subject: Re: [Codenarc-developer] (no subject)
Hi Artur,
Changing the phase worked with my test case to detect the interface once I
started using AstUtil.classNodeImplementsType for the check. The outstanding
question I had was whether I could skip doing the grails import of
org.codehaus.groovy.grails.commons.GrailsDomainClass by declaring a simple
GrailsDomainClass interface in the rule and checking for that. It looks like
some of the checks done by AstUtil.classNodeImplementsType may match on
simple class name while omitting the package name, which presumably would
work.
The other problem I ran into was how to run my custom CodeNarc rules in the
grails codenarc plugin. Any ideas here? I see comments saying it's not
trivially done.
Thanks
-Brian Soby
On Wed, Jan 29, 2014 at 2:06 PM, Artur Gajowy <artur.gajowy@...
<mailto:artur.gajowy@...> > wrote:
Did you change your rule's compilerPhase to SEMANTIC_ANALYSIS or greater? It
should detect the GrailsDomainClass interface in your test, however I'm not
sure if it would detect the implicit interface implementation done by
Grails.
Be aware of the limitations:
http://codenarc.sourceforge.net/codenarc-enhanced-classpath-rules.htmlhttps://github.com/CodeNarc/CodeNarc/pull/27https://github.com/CodeNarc/CodeNarc/issues/29
If, however, you're ok with running CodeNarc as a test in your project, the
classpath problems won't affect you. On the other hand - CodeNarc users
running it without Grails classes on classpath won't be able to benefit from
the rule.
If you decide to implement the rule - make sure you get the
GrailsDomainClass interface's package right and test it on a real project :)
Best regards,
Artur Gajowy
On 29 January 2014 19:21, Brian Soby <brian.soby@...
<mailto:brian.soby@...> > wrote:
Hi all,
I'm working on a mass assignment rule
(http://sourceforge.net/p/codenarc/feature-requests/392/) and I can't seem
to get type or interface based detection to work. Here's my test code:
public interface GrailsDomainClass {}
class Person implements GrailsDomainClass {
String name
Boolean isAdmin
}
params = [name: 'John', isAdmin: true]
def person = new Person(params)
Ideally, I'd 1) visit the Person constructor 2) Check that it implements
GrailsDomainClass 3) and check that the parameter implements java.util.Map.
Unfortunately, getInterfaces()/getAllInterfaces() seems to always return an
empty set. I've resorted to checking for a variable named "params" but that
seems too fragile. Is there anything I can do to have more robust type and
interface information available?
Thanks
-Brian Soby
----------------------------------------------------------------------------
--
WatchGuard Dimension instantly turns raw network data into actionable
security intelligence. It gives you real-time visual feedback on key
security issues and trends. Skip the complicated setup - simply import
a virtual appliance and go from zero to informed in seconds.
http://pubads.g.doubleclick.net/gampad/clk?id=123612991
<http://pubads.g.doubleclick.net/gampad/clk?id=123612991&iu=/4140/ostg.clktr
k> &iu=/4140/ostg.clktrk
_______________________________________________
Codenarc-developer mailing list
Codenarc-developer@...
<mailto:Codenarc-developer@...>
https://lists.sourceforge.net/lists/listinfo/codenarc-developer
No virus found in this message.
Checked by AVG - http://www.avg.com <http://www.avg.com&gt;
Version: 2014.0.4259 / Virus Database: 3684/7045 - Release Date: 01/30/14

I think you are running into the limitation of the AST compiler phase that CodeNarc uses by default. Take a look at:
http://codenarc.sourceforge.net/codenarc-rules-enhanced.html
CloneWithoutCloneableRule is an example of one of those "enhanced" rules:
https://github.com/CodeNarc/CodeNarc/blob/master/src/main/groovy/org/codenarc/rule/design/CloneWithoutCloneableRule.groovy
and sets
int compilerPhase = Phases.SEMANTIC_ANALYSIS
Just be aware of the limitation that the application classes/jars must be on the classpath when running CodeNarc. We are still trying to figure out the right way to integrate and expand the set of "enhanced" rules without breaking existing CodeNarc users that don't have that expanded classpath when running CodeNarc.
Chris
From: Brian Soby [mailto:brian.soby@...]
Sent: Wednesday, January 29, 2014 1:21 PM
To: codenarc-developer@...
Subject: [Codenarc-developer] (no subject)
Hi all,
I'm working on a mass assignment rule (http://sourceforge.net/p/codenarc/feature-requests/392/) and I can't seem to get type or interface based detection to work. Here's my test code:
public interface GrailsDomainClass {}
class Person implements GrailsDomainClass {
String name
Boolean isAdmin
}
params = [name: 'John', isAdmin: true]
def person = new Person(params)
Ideally, I'd 1) visit the Person constructor 2) Check that it implements GrailsDomainClass 3) and check that the parameter implements java.util.Map. Unfortunately, getInterfaces()/getAllInterfaces() seems to always return an empty set. I've resorted to checking for a variable named "params" but that seems too fragile. Is there anything I can do to have more robust type and interface information available?
Thanks
-Brian Soby

Re: running custom rules from the Grails CodeNarc plugin, unfortunately:
http://grails.org/plugin/codenarc
Note that your custom ruleset cannot refer to a custom rule defined within
the Grails application. This is due to a Grails classpath issue, discussed
here
<http://grails.1312388.n4.nabble.com/Can-a-plugin-use-an-application-level-c
lass-td3330014.html> .
It sounds like you could put the rule into another plugin or maybe a jar? I
know, not cool. :(
From: Brian Soby [mailto:brian.soby@...]
Sent: Wednesday, January 29, 2014 6:27 PM
To: Artur Gajowy
Cc: codenarc-developer@...
Subject: Re: [Codenarc-developer] (no subject)
Hi Artur,
Changing the phase worked with my test case to detect the interface once I
started using AstUtil.classNodeImplementsType for the check. The outstanding
question I had was whether I could skip doing the grails import of
org.codehaus.groovy.grails.commons.GrailsDomainClass by declaring a simple
GrailsDomainClass interface in the rule and checking for that. It looks like
some of the checks done by AstUtil.classNodeImplementsType may match on
simple class name while omitting the package name, which presumably would
work.
The other problem I ran into was how to run my custom CodeNarc rules in the
grails codenarc plugin. Any ideas here? I see comments saying it's not
trivially done.
Thanks
-Brian Soby
On Wed, Jan 29, 2014 at 2:06 PM, Artur Gajowy <artur.gajowy@...
<mailto:artur.gajowy@...> > wrote:
Did you change your rule's compilerPhase to SEMANTIC_ANALYSIS or greater? It
should detect the GrailsDomainClass interface in your test, however I'm not
sure if it would detect the implicit interface implementation done by
Grails.
Be aware of the limitations:
http://codenarc.sourceforge.net/codenarc-enhanced-classpath-rules.htmlhttps://github.com/CodeNarc/CodeNarc/pull/27https://github.com/CodeNarc/CodeNarc/issues/29
If, however, you're ok with running CodeNarc as a test in your project, the
classpath problems won't affect you. On the other hand - CodeNarc users
running it without Grails classes on classpath won't be able to benefit from
the rule.
If you decide to implement the rule - make sure you get the
GrailsDomainClass interface's package right and test it on a real project :)
Best regards,
Artur Gajowy
On 29 January 2014 19:21, Brian Soby <brian.soby@...
<mailto:brian.soby@...> > wrote:
Hi all,
I'm working on a mass assignment rule
(http://sourceforge.net/p/codenarc/feature-requests/392/) and I can't seem
to get type or interface based detection to work. Here's my test code:
public interface GrailsDomainClass {}
class Person implements GrailsDomainClass {
String name
Boolean isAdmin
}
params = [name: 'John', isAdmin: true]
def person = new Person(params)
Ideally, I'd 1) visit the Person constructor 2) Check that it implements
GrailsDomainClass 3) and check that the parameter implements java.util.Map.
Unfortunately, getInterfaces()/getAllInterfaces() seems to always return an
empty set. I've resorted to checking for a variable named "params" but that
seems too fragile. Is there anything I can do to have more robust type and
interface information available?
Thanks
-Brian Soby
----------------------------------------------------------------------------
--
WatchGuard Dimension instantly turns raw network data into actionable
security intelligence. It gives you real-time visual feedback on key
security issues and trends. Skip the complicated setup - simply import
a virtual appliance and go from zero to informed in seconds.
http://pubads.g.doubleclick.net/gampad/clk?id=123612991
<http://pubads.g.doubleclick.net/gampad/clk?id=123612991&iu=/4140/ostg.clktr
k> &iu=/4140/ostg.clktrk
_______________________________________________
Codenarc-developer mailing list
Codenarc-developer@...
<mailto:Codenarc-developer@...>
https://lists.sourceforge.net/lists/listinfo/codenarc-developer

Hi Artur,
Changing the phase worked with my test case to detect the interface once I
started using AstUtil.classNodeImplementsType for the check. The
outstanding question I had was whether I could skip doing the grails import
of org.codehaus.groovy.grails.commons.GrailsDomainClass by declaring a
simple GrailsDomainClass interface in the rule and checking for that. It
looks like some of the checks done by AstUtil.classNodeImplementsType may
match on simple class name while omitting the package name, which
presumably would work.
The other problem I ran into was how to run my custom CodeNarc rules in the
grails codenarc plugin. Any ideas here? I see comments saying it's not
trivially done.
Thanks
-Brian Soby
On Wed, Jan 29, 2014 at 2:06 PM, Artur Gajowy <artur.gajowy@...>wrote:
> Did you change your rule's compilerPhase to SEMANTIC_ANALYSIS or greater?
> It should detect the GrailsDomainClass interface in your test, however I'm
> not sure if it would detect the implicit interface implementation done by
> Grails.
>
> Be aware of the limitations:
> http://codenarc.sourceforge.net/codenarc-enhanced-classpath-rules.html
> https://github.com/CodeNarc/CodeNarc/pull/27
> https://github.com/CodeNarc/CodeNarc/issues/29
>
> If, however, you're ok with running CodeNarc as a test in your project,
> the classpath problems won't affect you. On the other hand - CodeNarc users
> running it without Grails classes on classpath won't be able to benefit
> from the rule.
>
> If you decide to implement the rule - make sure you get the
> GrailsDomainClass interface's package right and test it on a real project :)
>
> Best regards,
> Artur Gajowy
>
>
> On 29 January 2014 19:21, Brian Soby <brian.soby@...> wrote:
>
>> Hi all,
>>
>> I'm working on a mass assignment rule (
>> http://sourceforge.net/p/codenarc/feature-requests/392/) and I can't
>> seem to get type or interface based detection to work. Here's my test code:
>>
>> public interface GrailsDomainClass {}
>> class Person implements GrailsDomainClass {
>> String name
>> Boolean isAdmin
>> }
>> params = [name: 'John', isAdmin: true]
>> def person = new Person(params)
>>
>> Ideally, I'd 1) visit the Person constructor 2) Check that it implements
>> GrailsDomainClass 3) and check that the parameter implements java.util.Map.
>> Unfortunately, getInterfaces()/getAllInterfaces() seems to always return an
>> empty set. I've resorted to checking for a variable named "params" but that
>> seems too fragile. Is there anything I can do to have more robust type and
>> interface information available?
>>
>> Thanks
>> -Brian Soby
>>
>>
>>
>>
>> ------------------------------------------------------------------------------
>> WatchGuard Dimension instantly turns raw network data into actionable
>> security intelligence. It gives you real-time visual feedback on key
>> security issues and trends. Skip the complicated setup - simply import
>> a virtual appliance and go from zero to informed in seconds.
>>
>> http://pubads.g.doubleclick.net/gampad/clk?id=123612991&iu=/4140/ostg.clktrk
>> _______________________________________________
>> Codenarc-developer mailing list
>> Codenarc-developer@...
>> https://lists.sourceforge.net/lists/listinfo/codenarc-developer
>>
>>
>

Chris,
Sure! I just have to find some time to make the changes :) I can't really
promise anything, but will try to make them within a month.
I think you can't do much to help me for now, but thanks for asking!
Best regards,
Artur
On 4 July 2013 16:06, Chris Mair <chrismair@...> wrote:
> Artur,****
>
> ** **
>
> Are you still planning on submitting the changes for the inline violations
> support? Let me know if there is anything I can do to help. Thanks.****
>
> ** **
>
> Chris****
>
> ** **
>
> *From:* Chris Mair [mailto:chrismair@...]
> *Sent:* Friday, May 10, 2013 9:30 PM
> *To:* 'Hamlet DArcy'; 'Marcin Erdmann'
> *Cc:* 'codenarc-developer@...'
> *Subject:* RE: [Codenarc-developer] Inline violations idea****
>
> ** **
>
> I’m glad we are getting some discussion and feedback on this.****
>
> ** **
>
> I don’t think there is any question about breaking
> backwards-compatibility. Artur’s changes integrate with and are compatible
> with the existing APIs. We would continue to support new and old validation
> styles. I don’t think anyone is suggesting migrating all of the existing
> code to the new style (and potentially breaking other rule developers’
> code), but correct me if I’m wrong.****
>
> ** **
>
> I am still leaning toward a separate method for reasons including:****
>
> **· **This is a different style of validation; I think making
> that obvious is beneficial.****
>
> **· **I am worried about the potential for confusing the new and
> old styles of validations. For instance, a developer intends to use the new
> style , but neglects to annotate the source code with validations, and the
> test succeeds, even if it “should” not.****
>
> **· **There may be design/complexity advantages to keeping the
> new and old styles decoupled. That may give us more flexibility to extend
> or redesign later.****
>
> **· **It would just be adding a single protected method to the *
> AbstractRuleTestCase* API.****
>
> ** **
>
> Chris****
>
> ** **
>
> *From:* Hamlet DArcy [mailto:hamlet.darcy@...<hamlet.darcy@...>]
>
> *Sent:* Friday, May 10, 2013 3:22 AM
> *To:* Marcin Erdmann
> *Cc:* codenarc-developer@...
> *Subject:* Re: [Codenarc-developer] Inline violations idea****
>
> ** **
>
> The biggest issue is that the getLineNumber() method in Groovy is often
> returning the wrong line number. As long as the line number hacks in
> CodeNarc's AstUtils are well tested, then it seems like a reasonable
> change. However, I would not have two different public methods on the API.
> I don't think many projects depend on these test utilities, and it would be
> a lot cleaner to just have one way to make assertions rather than two. So I
> would go ahead and break backwards compatibility if we really do believe
> that this is a better API for testing. ****
>
> --
> Hamlet D'Arcy
> hamlet.darcy@...****
>
> ** **
> ------------------------------
>
> I'm a bit late to the party but I really like the idea. I never wrote too
> many rules but when I did line counting and passing those maps to
> assertViolations() was the biggest pain point in the otherwise terrific
> testing support for writing rules in Codenarc. So it would be nice if you
> pushed on and made all those changes required by Chris so that he has no
> choice but to merge it in. :)
>
> On 07/05/13 11:44, Chris Mair wrote:****
>
> Thanks Artur. That is a clever and innovative approach for validation of
> rule violations. Pretty cool! I acknowledge that configuring the violation
> line numbers and source line text and keeping them in sync after edits can
> be cumbersome. Your solution is a concise way to avoid that.****
>
> ****
>
> Some things to consider about the existing approach:****
>
> · The existing approach and associated validations are there
> intentionally to validate the exact line number in the generated Violation;
> line numbers are not always completely straightforward and automatically
> correct (especially around imports and annotations). Though the CodeNarc
> framework and helper classes generally make that a non-issue now, I would
> not want to skip line number validation for all rules/tests.****
>
> · The source line text is not guaranteed to be the full source
> line text; it may be a subset, or it may be null/empty. So, you could not
> generalize that it is always the full source line.****
>
> · I do like the way the existing approach allows a clean,
> standalone example of source code that causes violation. It can also be
> easily copied to the help documentation or pasted in an email (admittedly a
> minor benefit).****
>
> ****
>
> That all being said, I do think your approach has value. I would want to
> implement this as a separate method name, perhaps *assertInlineViolations*(),
> rather than adding complexity (and potentially parsing the source code?)
> for all invocations of *assertViolations*(). If you are willing to expand
> and harden your POC as you mentioned, I think we can pull it in.****
>
> ****
>
> Thanks.****
>
> Chris****
>
> ** **
>
> ------------------------------------------------------------------------------****
>
> Learn Graph Databases - Download FREE O'Reilly Book****
>
> "Graph Databases" is the definitive new guide to graph databases and ****
>
> their applications. This 200-page book is written by three acclaimed ****
>
> leaders in the field. The early access version is available now. ****
>
> Download your free book today! http://p.sf.net/sfu/neotech_d2d_may****
>
> ** **
>
> _______________________________________________****
>
> Codenarc-developer mailing list****
>
> Codenarc-developer@...****
>
> https://lists.sourceforge.net/lists/listinfo/codenarc-developer****
>
> _______________________________________________
> Codenarc-developer mailing list
> Codenarc-developer@...
> https://lists.sourceforge.net/lists/listinfo/codenarc-developer****
>
> ** **
>

Artur,
Are you still planning on submitting the changes for the inline violations support? Let me know if there is anything I can do to help. Thanks.
Chris
From: Chris Mair [mailto:chrismair@...]
Sent: Friday, May 10, 2013 9:30 PM
To: 'Hamlet DArcy'; 'Marcin Erdmann'
Cc: 'codenarc-developer@...'
Subject: RE: [Codenarc-developer] Inline violations idea
I’m glad we are getting some discussion and feedback on this.
I don’t think there is any question about breaking backwards-compatibility. Artur’s changes integrate with and are compatible with the existing APIs. We would continue to support new and old validation styles. I don’t think anyone is suggesting migrating all of the existing code to the new style (and potentially breaking other rule developers’ code), but correct me if I’m wrong.
I am still leaning toward a separate method for reasons including:
· This is a different style of validation; I think making that obvious is beneficial.
· I am worried about the potential for confusing the new and old styles of validations. For instance, a developer intends to use the new style , but neglects to annotate the source code with validations, and the test succeeds, even if it “should” not.
· There may be design/complexity advantages to keeping the new and old styles decoupled. That may give us more flexibility to extend or redesign later.
· It would just be adding a single protected method to the AbstractRuleTestCase API.
Chris
From: Hamlet DArcy [mailto:hamlet.darcy@...]
Sent: Friday, May 10, 2013 3:22 AM
To: Marcin Erdmann
Cc: codenarc-developer@...
Subject: Re: [Codenarc-developer] Inline violations idea
The biggest issue is that the getLineNumber() method in Groovy is often returning the wrong line number. As long as the line number hacks in CodeNarc's AstUtils are well tested, then it seems like a reasonable change. However, I would not have two different public methods on the API. I don't think many projects depend on these test utilities, and it would be a lot cleaner to just have one way to make assertions rather than two. So I would go ahead and break backwards compatibility if we really do believe that this is a better API for testing.
--
Hamlet D'Arcy
hamlet.darcy@...
_____
I'm a bit late to the party but I really like the idea. I never wrote too many rules but when I did line counting and passing those maps to assertViolations() was the biggest pain point in the otherwise terrific testing support for writing rules in Codenarc. So it would be nice if you pushed on and made all those changes required by Chris so that he has no choice but to merge it in. :)
On 07/05/13 11:44, Chris Mair wrote:
Thanks Artur. That is a clever and innovative approach for validation of rule violations. Pretty cool! I acknowledge that configuring the violation line numbers and source line text and keeping them in sync after edits can be cumbersome. Your solution is a concise way to avoid that.
Some things to consider about the existing approach:
· The existing approach and associated validations are there intentionally to validate the exact line number in the generated Violation; line numbers are not always completely straightforward and automatically correct (especially around imports and annotations). Though the CodeNarc framework and helper classes generally make that a non-issue now, I would not want to skip line number validation for all rules/tests.
· The source line text is not guaranteed to be the full source line text; it may be a subset, or it may be null/empty. So, you could not generalize that it is always the full source line.
· I do like the way the existing approach allows a clean, standalone example of source code that causes violation. It can also be easily copied to the help documentation or pasted in an email (admittedly a minor benefit).
That all being said, I do think your approach has value. I would want to implement this as a separate method name, perhaps assertInlineViolations(), rather than adding complexity (and potentially parsing the source code?) for all invocations of assertViolations(). If you are willing to expand and harden your POC as you mentioned, I think we can pull it in.
Thanks.
Chris
------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and
their applications. This 200-page book is written by three acclaimed
leaders in the field. The early access version is available now.
Download your free book today! http://p.sf.net/sfu/neotech_d2d_may
_______________________________________________
Codenarc-developer mailing list
Codenarc-developer@...
https://lists.sourceforge.net/lists/listinfo/codenarc-developer
_______________________________________________
Codenarc-developer mailing list
Codenarc-developer@...
https://lists.sourceforge.net/lists/listinfo/codenarc-developer

I’m glad we are getting some discussion and feedback on this.
I don’t think there is any question about breaking backwards-compatibility. Artur’s changes integrate with and are compatible with the existing APIs. We would continue to support new and old validation styles. I don’t think anyone is suggesting migrating all of the existing code to the new style (and potentially breaking other rule developers’ code), but correct me if I’m wrong.
I am still leaning toward a separate method for reasons including:
· This is a different style of validation; I think making that obvious is beneficial.
· I am worried about the potential for confusing the new and old styles of validations. For instance, a developer intends to use the new style , but neglects to annotate the source code with validations, and the test succeeds, even if it “should” not.
· There may be design/complexity advantages to keeping the new and old styles decoupled. That may give us more flexibility to extend or redesign later.
· It would just be adding a single protected method to the AbstractRuleTestCase API.
Chris
From: Hamlet DArcy [mailto:hamlet.darcy@...]
Sent: Friday, May 10, 2013 3:22 AM
To: Marcin Erdmann
Cc: codenarc-developer@...
Subject: Re: [Codenarc-developer] Inline violations idea
The biggest issue is that the getLineNumber() method in Groovy is often returning the wrong line number. As long as the line number hacks in CodeNarc's AstUtils are well tested, then it seems like a reasonable change. However, I would not have two different public methods on the API. I don't think many projects depend on these test utilities, and it would be a lot cleaner to just have one way to make assertions rather than two. So I would go ahead and break backwards compatibility if we really do believe that this is a better API for testing.
--
Hamlet D'Arcy
hamlet.darcy@...
_____
I'm a bit late to the party but I really like the idea. I never wrote too many rules but when I did line counting and passing those maps to assertViolations() was the biggest pain point in the otherwise terrific testing support for writing rules in Codenarc. So it would be nice if you pushed on and made all those changes required by Chris so that he has no choice but to merge it in. :)
On 07/05/13 11:44, Chris Mair wrote:
Thanks Artur. That is a clever and innovative approach for validation of rule violations. Pretty cool! I acknowledge that configuring the violation line numbers and source line text and keeping them in sync after edits can be cumbersome. Your solution is a concise way to avoid that.
Some things to consider about the existing approach:
· The existing approach and associated validations are there intentionally to validate the exact line number in the generated Violation; line numbers are not always completely straightforward and automatically correct (especially around imports and annotations). Though the CodeNarc framework and helper classes generally make that a non-issue now, I would not want to skip line number validation for all rules/tests.
· The source line text is not guaranteed to be the full source line text; it may be a subset, or it may be null/empty. So, you could not generalize that it is always the full source line.
· I do like the way the existing approach allows a clean, standalone example of source code that causes violation. It can also be easily copied to the help documentation or pasted in an email (admittedly a minor benefit).
That all being said, I do think your approach has value. I would want to implement this as a separate method name, perhaps assertInlineViolations(), rather than adding complexity (and potentially parsing the source code?) for all invocations of assertViolations(). If you are willing to expand and harden your POC as you mentioned, I think we can pull it in.
Thanks.
Chris
------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and
their applications. This 200-page book is written by three acclaimed
leaders in the field. The early access version is available now.
Download your free book today! http://p.sf.net/sfu/neotech_d2d_may
_______________________________________________
Codenarc-developer mailing list
Codenarc-developer@...
https://lists.sourceforge.net/lists/listinfo/codenarc-developer
_______________________________________________
Codenarc-developer mailing list
Codenarc-developer@...
https://lists.sourceforge.net/lists/listinfo/codenarc-developer

The biggest issue is that the getLineNumber() method in Groovy is often returning the wrong line number. As long as the line number hacks in CodeNarc's AstUtils are well tested, then it seems like a reasonable change. However, I would not have two different public methods on the API. I don't think many projects depend on these test utilities, and it would be a lot cleaner to just have one way to make assertions rather than two. So I would go ahead and break backwards compatibility if we really do believe that this is a better API for testing.
--
Hamlet D'Arcy
hamlet.darcy@...
----- Original Message -----
> I'm a bit late to the party but I really like the idea. I never wrote
> too many rules but when I did line counting and passing those maps
> to assertViolations() was the biggest pain point in the otherwise
> terrific testing support for writing rules in Codenarc. So it would
> be nice if you pushed on and made all those changes required by
> Chris so that he has no choice but to merge it in. :)
> On 07/05/13 11:44, Chris Mair wrote:
> > Thanks Artur. That is a clever and innovative approach for
> > validation
> > of rule violations. Pretty cool! I acknowledge that configuring the
> > violation line numbers and source line text and keeping them in
> > sync
> > after edits can be cumbersome. Your solution is a concise way to
> > avoid that.
>
> > Some things to consider about the existing approach:
>
> > · The existing approach and associated validations are there
> > intentionally to validate the exact line number in the generated
> > Violation; line numbers are not always completely straightforward
> > and automatically correct (especially around imports and
> > annotations). Though the CodeNarc framework and helper classes
> > generally make that a non-issue now, I would not want to skip line
> > number validation for all rules/tests.
>
> > · The source line text is not guaranteed to be the full source line
> > text; it may be a subset, or it may be null/empty. So, you could
> > not
> > generalize that it is always the full source line.
>
> > · I do like the way the existing approach allows a clean,
> > standalone
> > example of source code that causes violation. It can also be easily
> > copied to the help documentation or pasted in an email (admittedly
> > a
> > minor benefit).
>
> > That all being said, I do think your approach has value. I would
> > want
> > to implement this as a separate method name, perhaps
> > assertInlineViolations (), rather than adding complexity (and
> > potentially parsing the source code?) for all invocations of
> > assertViolations (). If you are willing to expand and harden your
> > POC as you mentioned, I think we can pull it in.
>
> > Thanks.
>
> > Chris
>
> > ------------------------------------------------------------------------------
>
> > Learn Graph Databases - Download FREE O'Reilly Book
>
> > "Graph Databases" is the definitive new guide to graph databases
> > and
>
> > their applications. This 200-page book is written by three
> > acclaimed
>
> > leaders in the field. The early access version is available now.
>
> > Download your free book today! http://p.sf.net/sfu/neotech_d2d_may
>
> > _______________________________________________
>
> > Codenarc-developer mailing list
> > Codenarc-developer@...
> > https://lists.sourceforge.net/lists/listinfo/codenarc-developer
>
> ------------------------------------------------------------------------------
> Learn Graph Databases - Download FREE O'Reilly Book
> "Graph Databases" is the definitive new guide to graph databases and
> their applications. This 200-page book is written by three acclaimed
> leaders in the field. The early access version is available now.
> Download your free book today! http://p.sf.net/sfu/neotech_d2d_may
> _______________________________________________
> Codenarc-developer mailing list
> Codenarc-developer@...
> https://lists.sourceforge.net/lists/listinfo/codenarc-developer

I'm a bit late to the party but I really like the idea. I never wrote
too many rules but when I did line counting and passing those maps to
assertViolations() was the biggest pain point in the otherwise terrific
testing support for writing rules in Codenarc. So it would be nice if
you pushed on and made all those changes required by Chris so that he
has no choice but to merge it in. :)
On 07/05/13 11:44, Chris Mair wrote:
>
> Thanks Artur. That is a clever and innovative approach for validation
> of rule violations. Pretty cool! I acknowledge that configuring the
> violation line numbers and source line text and keeping them in sync
> after edits can be cumbersome. Your solution is a concise way to avoid
> that.
>
> Some things to consider about the existing approach:
>
> ·The existing approach and associated validations are there
> intentionally to validate the exact line number in the generated
> Violation; line numbers are not always completely straightforward and
> automatically correct (especially around imports and annotations).
> Though the CodeNarc framework and helper classes generally make that a
> non-issue now, I would not want to skip line number validation for all
> rules/tests.
>
> ·The source line text is not guaranteed to be the full source line
> text; it may be a subset, or it may be null/empty. So, you could not
> generalize that it is always the full source line.
>
> ·I do like the way the existing approach allows a clean, standalone
> example of source code that causes violation. It can also be easily
> copied to the help documentation or pasted in an email (admittedly a
> minor benefit).
>
> That all being said, I do think your approach has value. I would want
> to implement this as a separate method name, perhaps
> *assertInlineViolations*(), rather than adding complexity (and
> potentially parsing the source code?) for all invocations of
> *assertViolations*(). If you are willing to expand and harden your POC
> as you mentioned, I think we can pull it in.
>
> Thanks.
>
> Chris
>
>
>
> ------------------------------------------------------------------------------
> Learn Graph Databases - Download FREE O'Reilly Book
> "Graph Databases" is the definitive new guide to graph databases and
> their applications. This 200-page book is written by three acclaimed
> leaders in the field. The early access version is available now.
> Download your free book today! http://p.sf.net/sfu/neotech_d2d_may
>
>
> _______________________________________________
> Codenarc-developer mailing list
> Codenarc-developer@...
> https://lists.sourceforge.net/lists/listinfo/codenarc-developer

To CodeNarc users and developers,
The next version of CodeNarc will include some new rules that require a
later compiler phase for the Groovy AST compiler. Instead of the default
CONVERSION phase, these rules will require the later SEMANTIC_ANALYSIS
phase. The benefit is that this will allow these rules to make use of a
richer and more complete Abstract Syntax Tree (AST). The downside is that
the later compiler phase requires CodeNarc to have the application classes
being analyzed, as well as any referenced classes, on the classpath.
NOTE: If a rule requiring a later compiler phase is included in the active
CodeNarc ruleset and enabled and one or more of the required classes is not
on the classpath, then CodeNarc will log a Log4J WARN message for each
source file that contains the missing references.
This new capability was proposed and implemented by Artur Gajowy. See Pull
request #14 <https://github.com/CodeNarc/CodeNarc/pull/14&gt; for details and
discussion.
CodeNarc users that use the Ant Task directly will need to adjust their Ant
scripts to expand the classpath, if they want to take advantage of these
special new rules.
I think that other tools/frameworks that use CodeNarc will not be able to
use these new rules initially, because the tool classpath will not support
it. The hope is that the other CodeNarc tools will eventually be enhanced to
provide the expanded classpath to CodeNarc - either optionally or always -
so that they can also take advantage of these new rules.
We also expect to continue to introduce more of these "special"
enhanced-classpath rules, since that greatly expands the capabilities for
CodeNarc rules.
Anyone who doesn't want to use the new rules or is unable to expand the
classpath as required, can just omit these special rules from the CodeNarc
ruleset or else disable the rules.
Thanks.
Chris

Thanks Artur. That is a clever and innovative approach for validation of
rule violations. Pretty cool! I acknowledge that configuring the violation
line numbers and source line text and keeping them in sync after edits can
be cumbersome. Your solution is a concise way to avoid that.
Some things to consider about the existing approach:
. The existing approach and associated validations are there
intentionally to validate the exact line number in the generated Violation;
line numbers are not always completely straightforward and automatically
correct (especially around imports and annotations). Though the CodeNarc
framework and helper classes generally make that a non-issue now, I would
not want to skip line number validation for all rules/tests.
. The source line text is not guaranteed to be the full source line
text; it may be a subset, or it may be null/empty. So, you could not
generalize that it is always the full source line.
. I do like the way the existing approach allows a clean, standalone
example of source code that causes violation. It can also be easily copied
to the help documentation or pasted in an email (admittedly a minor
benefit).
That all being said, I do think your approach has value. I would want to
implement this as a separate method name, perhaps assertInlineViolations(),
rather than adding complexity (and potentially parsing the source code?) for
all invocations of assertViolations(). If you are willing to expand and
harden your POC as you mentioned, I think we can pull it in.
Thanks.
Chris

Hi!
I think I've got an idea for a significant improvement of the (already
great) CodeNarc's test framework and would like to consult it before making
a pull request ;)
The thing that is (for me) the most cumbersome when writing tests is
passing the line number and line text to the assert*Violation[s] methods. I
think I understand the rationale behind having to pass both of them, which
seems to be a kind of a double check / 'checksum'. I'd like to propose an
API in which we only have to write the violation message - and the API
reliably finds the sourceLineText and lineNumber for us. I've made a POC
implementation in
inlineViolations<https://github.com/ArturGajowy/CodeNarc/tree/inlineViolations&gt;
branch
of my fork. The result is best depicted
here<https://github.com/ArturGajowy/CodeNarc/commit/9cb0777e9f1c14d4b532bbd43d0e84bea6ecf494#commitcomment-3147447&gt;
.
As you can see, the whole idea is based on violation markers placed in the
test sources - just in the line the violation takes place. This way, when
we add a line to the source before the violation (e.g. an import we've
missed or a class header, or another violating line) - we don't have to
edit any preexisting violation assertions. There's also no copy'n'paste and
line counting invlolved in the initial process of writing tests :)
The violation markers are removed before the analysis takes place, so that
the assertion errors do not become illegible and the markers don't
interfere with analysis made by rule under test.
As I've said - this is a POC. It lacks: support for multiple violations per
line, standalone tests, a bit of defensive programming and first of all -
your approval of the API :) Let me know what do you think of the idea: do
you like it, would you like this in CodeNarc, are there any issues to
address before you'd like a pull request with that.
And of course - I'll be more than happy to implement the final version :)
Best regards,
Artur Gajowy

Community

Help

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:

CountryState

JavaScript is required for this form.

I agree to receive quotes, newsletters and other information from sourceforge.net and its partners regarding IT services and products. I understand that I can withdraw my consent at any time. Please refer to our Privacy Policy or Contact Us for more details