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