[CI]: static analysis checks #2400
Conversation
|
This pull request is now in conflicts. Could you fix it @m-ildefons? 🙏 |
There was a problem hiding this comment.
From the doc https://staticcheck.dev/docs/running-staticcheck/cli/
At its core, the staticcheck command works a lot like go vet or go build
Could you give a big explanation of why we need the staticcheck instead of using go vet or go build?
Install and run static analysis checker by default. The tool in use is `staticcheck` and it runs with all default options except rule SA1019 (using deprecated functions, variables, constants or fields). Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
Remove unused `init` function. Since go 1.20, seeding the random number generator is not necessary anyways. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
Eliminate unnecessary loop in `ListPodsBySelectorRO` Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
Remove unused code Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
- Remove unused function from admission webhook server: `decodeObjects` is not needed as the same purpose is already fulfilled by `Request.DecodeObjects` - Replace in-place object construction with use of `statusErrorWithMessage` - Remove unused variables `failPolicyIgnore`, `matchPolicyEquivalent` and `longhornFinalizerKey` - Remove use of `fmt.Sprintf` with constant strings - Remove superfluous `return` at end of function `ServeHTTP` Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
- Remove unused function `getNodesWithEvictingReplicas` - remove superfluous check for map value. If the accessed array doesn't yet exist, `append` will create it automatically. No need to manually specify it Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
Add pre-commit config for auto checking files before submitting to CI. Pre-commit checks with hooks for spelling mistakes, end-of-file and other whitespace errors, not just Go code, but also readme files, yaml and more. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
- Remove unused variable `maxRetriesOnAcquireLockError` This is no longer necessary since the rate-limited retry function will retry for more than 150 seconds anyways - Change use of `fmt.Errorf` to `errors.Wrap` Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
- Remove unused assignments from controller tests. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
- Remove unnecessary parameter `err` from `handleStatusUpdate` This parameter was taking an error by value but the function was not using this error in any way before reassigning the variable `err`. Therefore in all cases the value taking in as parameter was discarded, making it unnecessary to pass the parameter in the first place. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
- Remove unnecessary assignments to variables that aren't used later anymore - Remove function `isVolumeAvailableOnNode` from manager, since it has been moved to the attach/detach controller along with the application of it. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
Remove dead code from recurring job module: `deleteSnapshots` and `waitForVolumeState`, two functions that aren't called anywhere Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
Check all return values Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
- Set the compressionMethod field in the constructor - Fix error message being in the wrong case Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
- Remove redundant type information in array construction - Check error return value when listing nodes - Check error return value of anonymous function and propagate the error up - Remove dead code Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
Declare variable without allocation to avoid wasteful re-allocation during later assignment. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
- Remove unused variable Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
Avoid assigning to variable that isn't used later in the function Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
- Remove unused functions - utilize `isDataLocalityStrictLocal` instead of ad-hoc comparisons Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
- Avoid reassigning variable that isn't used later in the function Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
- utilize `deleteDeployment` function instead of ad-hoc code doing the same - remove unused function `getLoggerForUninstallService` Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
Remove unused function `getVolumeBackupName` Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
Enforce static analysis during validation. The script will now return a non-zero return code when the static analysis tool finds something to complain about Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
- check return value `err` in controller/controller_test.go - avoid reassigning `obj` in datastore/longhorn.go related-to: longhorn/longhorn#7300 Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
m-ildefons
force-pushed
the
static-analysis
branch
from
0ce4efe
to
22a5f8c
Compare
Remove field `compressionMethod` as it is unused. Signed-off-by: Moritz Röhrich <moritz.rohrich@suse.com>
|
@PhanLe1010 the documentation you quoted is talking about how the command line interface is similar to
I hope this answers your question. |
@m-ildefons will you be including the issue numbers in each commit message? I suggest not using |
|
This pull request is now in conflict. Could you fix it @m-ildefons? 🙏 |
|
@m-ildefons |
|
This pull request is now in conflict. Could you fix it @m-ildefons? 🙏 |
|
@FrankYang0529 Can you help review this PR and see if we can close the PR? I think most of the fixes have been included in your golint fix PR. cc @innobead |
Introduce static analysis into the validation step of the build process, using the tool
staticcheckFix problem areas found by
staticcheck. Most of the fixes are dead code removals and style issues, but there are also some potential nil dereferences and code duplication issues.The fixes are broken up into many small commits making it easier to review and validate them.
Related to: longhorn/longhorn#7300