analyzer: fix int-conversion scope

[ccoVeille]

intConv parameter was introduced to disable some rules.

The code was introduced with e958370, but only a part of the rules
related to integer conversions were altered.

This commit completes what was done in the previous commit

[ccoVeille]

Here is a suggestion I'm doing after trying to play with the config to disable some of the rules.

Please review, do not hesitate to tell me if I'm wrong with this idea. Thanks

cc @alexandear as I'm curious what he would think about it

[catenacyber]

Well, intConv was meant to be about integer conversion like int64(a) because it is less nice/readable code.

Why do you want to disable fmt.Sprintf to strconv.Itoa ?
If you really want, I guess it should be a new option

[ccoVeille]

The idea is to be able to be able to disable everything via the configuration.

If someone only wants errrof and sprintf1, it should be possible.

I chose existing intConv variable because it feels like it was also about int conversion.

But maybe you are right. Also having a new variable will avoid breaking changes for people.

What would be the name of the variable according to you?

It's about integer formating, so maybe intFormat?

[ccoVeille]

Just noticing with work @mmorel-35 here

#31 (comment)

That at least format bool would need a configuration variable too

[alexandear]

It's about integer formating, so maybe intFormat?

It can be intFormat.

So, let's change this PR to add a new option field to the perfSprint struct.

[alexandear]

I propose adding new option instead of reusing existing intConv.

[alexandear]

The idea is to be able to be able to disable everything via the configuration.

@catenacyber to be more clear, we want something that is already present in govet or revive. These linters have options to turn off or turn on specific checkers or rules.

Take a look
https://github.com/golangci/golangci-lint/blob/4c04e0053f6e03920293ddd5a3c4750410d8cbc3/.golangci.reference.yml#L1741

https://github.com/golangci/golangci-lint/blob/4c04e0053f6e03920293ddd5a3c4750410d8cbc3/.golangci.reference.yml#L2306

[ccoVeille]

I propose adding new option instead of reusing existing intConv.

I agree. I'll use intFormat, add boolFormat and any other needed.

I'll also update the tests because noconv should contain them all (at least FormatBool is missing)

[catenacyber]

I agree. I'll use intFormat, add boolFormat and any other needed.

Sounds good, waiting for this

[ccoVeille]

Based on the changes made on

• chore: refactor Diagnostic declaration #31

I'll wait it to be merged before working again on this PR.

The number of changes in the testdata will be crazy, I'll face too many conflicts.

So, let's wait

[ccoVeille]

I'm now waiting for #38 to be merged

Then I can open a new PR.

@mmorel-35 do you plan to update all the tests the checker names (the one in TODO for now), or do you want me to do it in my PR ?

[mmorel-35]

I don't plan it yet we need a clear list of what are the checkers name and what are the options first

[ccoVeille]

OK, so I can do it with my PR, as I will need to add configuration option for them.

[mmorel-35]

For example it could be :

error-format

❌
fmt.Errorf("Simple message")
fmt.Sprintf("%s", err)
fmt.Sprintf("%v", err)

✅
errors.New("Simple message")
err.Error()

with:

• error-format.err-error option equivalent to err-error to use err.Error() even if it is only equivalent for non-nil errors

string-format

❌
fmt.Sprintf("%s", strVal)
fmt.Sprintf("format string %s", strVal)
fmt.Sprintf("%s format string", strVal)

✅
strVal
"format string " + strVal
strVal + "format string"

with

• string-format.concat option equivalent to strconcat to enable strings concatenation

type-format

❌
fmt.Sprintf("%t", boolVal)
fmt.Sprintf("%x", hash)
fmt.Sprintf("%d", id)
fmt.Sprintf("%v", version)

✅
strconv.FormatBool(boolBal)
hex.EncodeToString(hash)
strconv.Itoa(id)
strconv.FormatUint(uint64(version), 10)

with

• type-format.int-conversion option equivalent to int-conversion to enable int or uint type cast

[ccoVeille]

Yes, exactly.

We also talked about bool-format

For the int-conv of int format I don't know.

But these are the things that will be reviewed when code is ready

[ccoVeille]

But maybe the simplest would be for you to open a PR once #38 will be merged to add these checker name.

The things I'm doing in this PR can be addressed later

[mmorel-35]

It's probably simpler for everyone to have a dedicated issue to work what has to be done first

[mmorel-35]

I just created #39, please have a look at it so we can define clearly how to adapt the structuration of the code