Sonarcloud optimizations

[ineiti]

More optimizations for the optimizations:

• Ask your admin to set a New Code definition.
• Change the sonarcloud rule functions should match the regular expression ^(_|[a-zA-Z0-9]+)$

[ineiti]

Asked @pierluca for admin accesss on sonarcloud dedis/cothority

[ineiti]

Finally found the menu where they are stored. Changed it to:

^(_|[a-zA-Z0-9]+|Test[A-Z][a-zA-Z0-9]+_[a-zA-Z0-9]+)$

[tharvik]

You accept a single underscore here, but for example, in blscosi/protocol/protocol_test.go, three are sometimes used.
The one below looks better to me but I didn't test it.

^(_|Test[A-Z])[a-zA-Z0-9_]+$

[ineiti]

Hmm - that one allows _hello_ and I'm not fan of the _5_9 in the test-name - those should be done using a for-loop or so... What about

^(_|Test[a-zA-Z0-9_]+)$

which is more readable, too?

[tharvik]

Hmm - that one allows _hello_ and I'm not fan of the _5_9 in the test-name - those should be done using a for-loop or so...

I agree, that's not really nice.

What about

^(_|Test[a-zA-Z0-9_]+)$

It also allows for TestmyMethod which is not really readable either, I'd keep the capital letter after "Test". And it also disallow test helpers.

Something like the following? It also ensure a single underscore and a capital after it

^(Test[A-Z]|[a-z])[a-zA-Z0-9]*(_[A-Z][a-zA-Z0-9]*)?$

(arf, it's getting uglier by the minute)

NB: I don't quite agree with the functions starting by an underscore, IMO it's better to use a minuscule

[ineiti]

OK, we're getting there. Currently you disallow all public methods, which is a bit harsh. I know you prefer not to expose too many methods, but none? Sorry, couldn't resist...

Also, if we suppose that it is Test{structure}_{method}, we cannot assume neither an uppercase letter in structure, nor in method...

NB: I don't quite agree with the functions starting by an underscore, IMO it's better to use a minuscule

If my regexp-brain-centre is up-to-date, ^(_|[a-zA-Z0-9]+)$ means a single _ as function name, without anything else...

All in all, I think the ^(_|Test[a-zA-Z0-9_]+)$ is the least bad. I'd prefer ^(_|Test[a-zA-Z0-9]+_[a-zA-Z0-9]+)$, but that would exclude the Test.*_5_9 names. Which probably should be excluded ;)

[tharvik]

OK, we're getting there. Currently you disallow all public methods, which is a bit harsh. I know you prefer not to expose too many methods, but none? Sorry, couldn't resist...

Hoo right, I'm rereading the issue and the regex is not only for the tests, that's why 😅

Also, if we suppose that it is Test{structure}_{method}, we cannot assume neither an uppercase letter in structure, nor in method...

Hum, make sense then, a bit hard to read but valid.

NB: I don't quite agree with the functions starting by an underscore, IMO it's better to use a minuscule

If my regexp-brain-centre is up-to-date, ^(_|[a-zA-Z0-9]+)$ means a single _ as function name, without anything else...

Exact, which I never saw anywhere and would be quite unreadable (maybe even uncallable?); why not remove it?

All in all, I think the ^(_|Test[a-zA-Z0-9_]+)$ is the least bad. I'd prefer ^(_|Test[a-zA-Z0-9]+_[a-zA-Z0-9]+)$, but that would exclude the Test.*_5_9 names. Which probably should be excluded ;)

Soo, as that's for the whole code and not only the test packages, we only accept public functions beginning with "Test"..

I'm going deeper in the rabbit hole, because I only want to support underscores inside test packages, not elsewhere

^(
    [^T][^e][^s][^t][a-zA-Z0-9]  // accept all functions not starting by "Test" and no underscore (if you know a better way to exclude a string...)
    |Test[a-zA-Z0-9]+            // or starting by "Test" and no underscore
        (_[a-zA-Z0-9]+)?         // and potentially having the `_{method}` at the end
)$

(damn you regex..)

[ineiti]

Soo, as that's for the whole code and not only the test packages, we only accept public functions beginning with "Test"..

Oh dang - I log off. My regexp-part of the brain is shutting down...

Why test for no Test? Isn't the following enough?

^(
    [a-zA-Z0-9]+                   // accept all functions with alphanumeric names, including `TestabCd`
    |Test[a-zA-Z0-9]+            // or starting by "Test", some alphanumerics,
        _[a-zA-Z0-9]+            // and the `_{method}` at the end
)$

[tharvik]

Right! That's way better! I think it's the right one 😄

I'm using find -name '*.go' -execdir grep -h '^func' {} + | grep -vE '^func( \([^)]+\))? ([a-zA-Z0-9]+|Test[a-zA-Z0-9]+_[a-zA-Z0-9]+)\(' to test what is not matching and it yields only the mulitple-undescores and the "Test_" functions, which is what we wanted.