Re: [Codenarc-developer] ClosureAsLastMethodParameterRule - bug with double slashes; simplified?

[Chris M. <chr...@ea...>]

I pulled in the test changes from the patch (and checked it in) and the code
still passes all tests.

Chris
-----Original Message-----
From: Hamlet D'Arcy [mailto:ham...@gm...] 
Sent: Friday, June 03, 2011 2:13 AM
To: Chris Mair
Cc: cod...@li...
Subject: Re: ClosureAsLastMethodParameterRule - bug with double slashes;
simplified?

Shoot, I also have this patch to apply from Marcin the original author.

I won't have time soon, but we should rip the tests out of the patch and see
if they still pass with your changes.



On Fri, Jun 3, 2011 at 4:12 AM, Chris Mair <chr...@ea...> wrote:
> Hamlet,
>
>
>
> When running the CodeNarc 0.14 snapshot against a project at work, I 
> ran across a bug in ClosureAsLastMethodParameterRule, where it would 
> give a false-positive violation if the closure parameter contained a 
> string containing “//”.
>
>
>
> Here is the test case I added, that initially failed with three
violations:
>
>
>
>     void testLinesWithStringsContainingDoubleSlash() {
>
>         final SOURCE = '''
>
>             void testProcess() {
>
>                 shouldFail { 
> process('xxx://wrsnetdev.usa.wachovia.net') }
>
>                 shouldFail(Exception) { process('http://')
>
>                 }
>
>                 shouldFail {
>
>                     process('http://wrsnetdev.usa.wachovia.net:xxx') }
>
>             }
>
>         '''
>
>         assertNoViolations(SOURCE)
>
>     }
>
>
>
> I stripped out the logic in that rule that checked for comments in the 
> source, and just base the check on the lastColumnNumber of the closure 
> parm versus the whole method. It may be too simplistic, but it passes 
> all of the tests, including my new test.  If you get a chance, could 
> you please take a look. If I am missing some “requirement”, then we 
> obviously need more/better tests.
>
>
>
> Chris



--
Hamlet D'Arcy
ham...@gm...