codenarc-developer Mailing List for CodeNarc
Brought to you by:
chrismair
This list is closed, nobody may subscribe to it.
2010 |
Jan
|
Feb
|
Mar
|
Apr
(8) |
May
(17) |
Jun
|
Jul
|
Aug
|
Sep
|
Oct
(8) |
Nov
(67) |
Dec
(9) |
---|---|---|---|---|---|---|---|---|---|---|---|---|
2011 |
Jan
(23) |
Feb
(19) |
Mar
(15) |
Apr
(7) |
May
(5) |
Jun
(43) |
Jul
(5) |
Aug
(11) |
Sep
(18) |
Oct
(9) |
Nov
(6) |
Dec
|
2012 |
Jan
(7) |
Feb
(2) |
Mar
(7) |
Apr
|
May
|
Jun
|
Jul
(17) |
Aug
(5) |
Sep
|
Oct
(1) |
Nov
|
Dec
(1) |
2013 |
Jan
|
Feb
(1) |
Mar
(7) |
Apr
|
May
(6) |
Jun
|
Jul
(2) |
Aug
|
Sep
|
Oct
|
Nov
|
Dec
|
2014 |
Jan
(6) |
Feb
(3) |
Mar
|
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
|
Oct
|
Nov
|
Dec
|
2016 |
Jan
(2) |
Feb
|
Mar
|
Apr
(2) |
May
|
Jun
|
Jul
|
Aug
|
Sep
|
Oct
|
Nov
|
Dec
|
2017 |
Jan
|
Feb
|
Mar
(4) |
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
|
Oct
|
Nov
|
Dec
|
From: Chris M. <cm...@gm...> - 2017-03-05 20:53:40
|
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 <mar...@pr...> 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 <cm...@gm...> 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 <mar...@pr... >> > 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 >>> Cod...@li... >>> https://lists.sourceforge.net/lists/listinfo/codenarc-developer >>> >>> >> > |
From: Marcin E. <mar...@pr...> - 2017-03-05 19:03:45
|
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 <cm...@gm...> 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 <mar...@pr...> > 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 >> Cod...@li... >> https://lists.sourceforge.net/lists/listinfo/codenarc-developer >> >> > |
From: Chris M. <cm...@gm...> - 2017-03-04 02:15:14
|
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 <mar...@pr...> 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 > Cod...@li... > https://lists.sourceforge.net/lists/listinfo/codenarc-developer > > |
From: Marcin E. <mar...@pr...> - 2017-03-03 14:11:57
|
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 |
From: Chris M. <cm...@gm...> - 2016-04-07 20:57:44
|
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 |
From: Naveena P. <puj...@gm...> - 2016-04-05 22:25:14
|
Hi, I want to develop new rules using MyEclipse 8.6.1 and I am using codeNarc 1.9 for Gosu. Could some one help me with MyEclipse Thanks naveena |
From: <chr...@we...> - 2016-01-08 16:56:55
|
CodeNarc is now built using Gradle<http://gradle.org/>, not Maven. Chris From: SoftwareEngineering Hauschel [mailto:in...@ha...] Sent: Friday, January 08, 2016 7:35 AM To: cod...@li... 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 |
From: SoftwareEngineering H. <in...@ha...> - 2016-01-08 12:34:37
|
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 |
From: Chris M. <chr...@ea...> - 2014-02-04 23:02:49
|
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:bri...@ta...] Sent: Monday, February 03, 2014 2:41 PM To: Chris Mair Cc: Artur Gajowy; cod...@li... 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_vulnerability http://www.infoq.com/news/2012/03/GitHub-Compromised Thanks -Brian Soby On Sun, Feb 2, 2014 at 7:25 AM, Chris Mair <chr...@ea... <mailto:chr...@ea...> > 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> , 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:bri...@ta... <mailto:bri...@ta...> ] Sent: Thursday, January 30, 2014 4:08 PM To: Chris Mair Cc: Artur Gajowy; cod...@li... <mailto:cod...@li...> 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 <chr...@ea... <mailto:chr...@ea...> > 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:bri...@ta... <mailto:bri...@ta...> ] Sent: Wednesday, January 29, 2014 6:27 PM To: Artur Gajowy Cc: cod...@li... <mailto:cod...@li...> 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 <art...@gm... <mailto:art...@gm...> > 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 <bri...@ta... <mailto:bri...@ta...> > 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 Cod...@li... <mailto:Cod...@li...> https://lists.sourceforge.net/lists/listinfo/codenarc-developer No virus found in this message. Checked by AVG - www.avg.com <http://www.avg.com> Version: 2014.0.4259 / Virus Database: 3684/7045 - Release Date: 01/30/14 No virus found in this message. Checked by AVG - www.avg.com <http://www.avg.com> Version: 2014.0.4259 / Virus Database: 3684/7054 - Release Date: 02/02/14 |
From: Brian S. <bri...@ta...> - 2014-02-03 19:41:13
|
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_vulnerability http://www.infoq.com/news/2012/03/GitHub-Compromised Thanks -Brian Soby On Sun, Feb 2, 2014 at 7:25 AM, Chris Mair <chr...@ea...> 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>, > 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:bri...@ta...] > *Sent:* Thursday, January 30, 2014 4:08 PM > *To:* Chris Mair > *Cc:* Artur Gajowy; cod...@li... > *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 <chr...@ea...> > 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>.* > > > > It sounds like you could put the rule into another plugin or maybe a jar? > I know, not cool... L > > > > *From:* Brian Soby [mailto:bri...@ta...] > *Sent:* Wednesday, January 29, 2014 6:27 PM > *To:* Artur Gajowy > *Cc:* cod...@li... > *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 <art...@gm...> > 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 <bri...@ta...> 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 > Cod...@li... > https://lists.sourceforge.net/lists/listinfo/codenarc-developer > > > > > > > > No virus found in this message. > Checked by AVG - www.avg.com > Version: 2014.0.4259 / Virus Database: 3684/7045 - Release Date: 01/30/14 > |
From: Chris M. <chr...@ea...> - 2014-02-02 15:26:01
|
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> , 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:bri...@ta...] Sent: Thursday, January 30, 2014 4:08 PM To: Chris Mair Cc: Artur Gajowy; cod...@li... 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 <chr...@ea... <mailto:chr...@ea...> > 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:bri...@ta... <mailto:bri...@ta...> ] Sent: Wednesday, January 29, 2014 6:27 PM To: Artur Gajowy Cc: cod...@li... <mailto:cod...@li...> 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 <art...@gm... <mailto:art...@gm...> > 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 <bri...@ta... <mailto:bri...@ta...> > 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 Cod...@li... <mailto:Cod...@li...> https://lists.sourceforge.net/lists/listinfo/codenarc-developer No virus found in this message. Checked by AVG - www.avg.com <http://www.avg.com> Version: 2014.0.4259 / Virus Database: 3684/7045 - Release Date: 01/30/14 |
From: Brian S. <bri...@ta...> - 2014-01-30 21:08:32
|
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 <chr...@ea...> 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>.* > > > > It sounds like you could put the rule into another plugin or maybe a jar? > I know, not cool... L > > > > *From:* Brian Soby [mailto:bri...@ta...] > *Sent:* Wednesday, January 29, 2014 6:27 PM > *To:* Artur Gajowy > *Cc:* cod...@li... > *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 <art...@gm...> > 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 <bri...@ta...> 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 > Cod...@li... > https://lists.sourceforge.net/lists/listinfo/codenarc-developer > > > > > |
From: <chr...@we...> - 2014-01-30 02:26:42
|
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:bri...@ta...] Sent: Wednesday, January 29, 2014 1:21 PM To: cod...@li... 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 |
From: Chris M. <chr...@ea...> - 2014-01-30 00:15:59
|
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:bri...@ta...] Sent: Wednesday, January 29, 2014 6:27 PM To: Artur Gajowy Cc: cod...@li... 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 <art...@gm... <mailto:art...@gm...> > 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 <bri...@ta... <mailto:bri...@ta...> > 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 Cod...@li... <mailto:Cod...@li...> https://lists.sourceforge.net/lists/listinfo/codenarc-developer |
From: Brian S. <bri...@ta...> - 2014-01-29 23:27:02
|
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 <art...@gm...>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 <bri...@ta...> 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 >> Cod...@li... >> https://lists.sourceforge.net/lists/listinfo/codenarc-developer >> >> > |
From: Artur G. <art...@gm...> - 2014-01-29 22:07:04
|
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 <bri...@ta...> 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 > Cod...@li... > https://lists.sourceforge.net/lists/listinfo/codenarc-developer > > |
From: Brian S. <bri...@ta...> - 2014-01-29 18:45:23
|
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 |
From: Artur G. <art...@gm...> - 2013-07-05 10:36:16
|
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 <chr...@ea...> 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:chr...@ea...] > *Sent:* Friday, May 10, 2013 9:30 PM > *To:* 'Hamlet DArcy'; 'Marcin Erdmann' > *Cc:* 'cod...@li...' > *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:ham...@ca...<ham...@ca...>] > > *Sent:* Friday, May 10, 2013 3:22 AM > *To:* Marcin Erdmann > *Cc:* cod...@li... > *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 > ham...@ca...**** > > ** ** > ------------------------------ > > 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**** > > Cod...@li...**** > > https://lists.sourceforge.net/lists/listinfo/codenarc-developer**** > > _______________________________________________ > Codenarc-developer mailing list > Cod...@li... > https://lists.sourceforge.net/lists/listinfo/codenarc-developer**** > > ** ** > |
From: Chris M. <chr...@ea...> - 2013-07-04 14:06:55
|
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:chr...@ea...] Sent: Friday, May 10, 2013 9:30 PM To: 'Hamlet DArcy'; 'Marcin Erdmann' Cc: 'cod...@li...' 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:ham...@ca...] Sent: Friday, May 10, 2013 3:22 AM To: Marcin Erdmann Cc: cod...@li... 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 ham...@ca... _____ 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 Cod...@li... https://lists.sourceforge.net/lists/listinfo/codenarc-developer _______________________________________________ Codenarc-developer mailing list Cod...@li... https://lists.sourceforge.net/lists/listinfo/codenarc-developer |
From: Chris M. <chr...@ea...> - 2013-05-11 01:29:48
|
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:ham...@ca...] Sent: Friday, May 10, 2013 3:22 AM To: Marcin Erdmann Cc: cod...@li... 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 ham...@ca... _____ 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 Cod...@li... https://lists.sourceforge.net/lists/listinfo/codenarc-developer _______________________________________________ Codenarc-developer mailing list Cod...@li... https://lists.sourceforge.net/lists/listinfo/codenarc-developer |
From: Hamlet D. <ham...@ca...> - 2013-05-10 07:22:06
|
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 ham...@ca... ----- 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 > > Cod...@li... > > 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 > Cod...@li... > https://lists.sourceforge.net/lists/listinfo/codenarc-developer |
From: Marcin E. <mar...@pr...> - 2013-05-09 18:17:31
|
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 > Cod...@li... > https://lists.sourceforge.net/lists/listinfo/codenarc-developer |
From: Chris M. <chr...@ea...> - 2013-05-08 01:10:58
|
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> 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 |
From: Chris M. <chr...@ea...> - 2013-05-07 10:44:18
|
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 |
From: Artur G. <art...@gm...> - 2013-05-05 19:44:33
|
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> branch of my fork. The result is best depicted here<https://github.com/ArturGajowy/CodeNarc/commit/9cb0777e9f1c14d4b532bbd43d0e84bea6ecf494#commitcomment-3147447> . 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 |