ignoreNamedArguments breaks marking non-named magic number params

[lwasylkowski]

The following test added in MagicNumberSpec fails:

given("ignoring named arguments is off") {

    fun code(numberString: String) = compileContentForTest("""
        data class Model(
                val someVal: Int,
                val other: String = "default"
        )

        var model = Model($numberString)
    """)

    it("should not ignore int") {
        val rule = MagicNumber(TestConfig(mapOf("ignoreNamedArgument" to "true")))
        assertThat(rule.lint(code("53"))).hasSize(1)
    }
}

It looks like this behaviour is not correct, since Model($numberString) will resolve to a non-named argument being passed a magic number. ignoreNamedArguments option set to true should not affect lint result in this case, but it does. In our codebase ignoreNamedArgument: true makes magic numbers like this:

internal fun String.appendRandomDigit() = plus(Random().nextInt(10))

go undetected

[arturbosch]

How old is your branch? Your not on the master branch, are you?
There is no such test case in master.

	given("ignoring named arguments") {
		given("in constructor invocation") {
			fun code(numberString: String) = compileContentForTest("""
				data class Model(
						val someVal: Int,
						val other: String = "default"
				)

				var model = Model(someVal = $numberString)
			""")

			it("should not ignore int by default") {
				assertThat(MagicNumber().lint(code("53"))).hasSize(1)
			}
....

Sorry for the late response.

[lwasylkowski]

Right, there's no such test - I've added it to show that the rule is behaving incorrectly.

The test you've pasted checks that if ignoreNamedArgument = false then named number is not ignored.

The test I've added checks whether if ignoreNamedArgument = true then unnamed number is catched, but it fails. But ignoreNamedArgument should have no effect on unnamed arguments, right? Essentially once someone sets ignoreNamedArguments = true in detekt config, then all number parameters are ignored by the magic number rule, named or not

[arturbosch]

Oh, misunderstood. Yes, your right. I verified the wrong behaviour with your test case.

Edit: Thanks for reporting. Fixed.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related topics.