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

[Hamlet D'A. <ham...@gm...>]

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...