Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve code analysis CI in Gaffer #2722

Closed
t92549 opened this issue Jul 29, 2022 · 5 comments · Fixed by #2871
Closed

Improve code analysis CI in Gaffer #2722

t92549 opened this issue Jul 29, 2022 · 5 comments · Fixed by #2871
Assignees
Labels
automation GitHub Actions, CI/CD

Comments

@t92549
Copy link
Contributor

t92549 commented Jul 29, 2022

#39 added FindBugs to Gaffer. FindBugs is no longer maintained and has been replaced by SpotBugs.

This could be upgraded, or a more modern tool could be used to replace this entirely.
For example:

Some of these would also replace the need for other plugins such as checkstyle and code coverage as they handle those too.

@t92549 t92549 added the automation GitHub Actions, CI/CD label Jul 29, 2022
@t92549 t92549 added this to the v2.0.0 milestone Jul 29, 2022
@lb324567
Copy link
Member

lb324567 commented Aug 31, 2022

Some work has been done on this ticket to identify the best tools going forward, but further work on this ticket is currently on hold. For more information see internal tool- number 301

@lb324567
Copy link
Member

SonarQube is the likely solution to this, but will need checking when this ticket is progressed. For more information see internal tool- number 301

@GCHQDeveloper314
Copy link
Member

#2842 will upgrade our SpotBugs configuration and improve code quality standards.
We might want to consider using the PMD source code analyser which also runs as a maven plugin and works well alongside SpotBugs.
We could also consider enabling the Google Java style formatting check in the Spotless plugin.

@GCHQDeveloper314
Copy link
Member

#2871 introduces the PMD plugin which performs further static analysis and code quality checks. It also enables GitHub's CodeQL security scanning. This issue can probably be closed once this has been merged.

To an extent these extra checks replace the checkstyle plugin, although they don't replace the code style checks. These could be replaced by the Spotless plugin formatting checks mentioned above, but this would also require major refactors to get the codebase to comply and so it's probably easier to leave this for now.

GCHQDeveloper314 added a commit that referenced this issue Feb 8, 2023
* Add PMD plugin to build execution and site reporting

* Fixes for common-util

* Further fixes

* Add PMD Rules file and expanded checks

* Remove LocalVariableCouldBeFinal rule
This will require too many changes to some classes which should just be refactored

* Remove ConfusingTernary, requires too many changes to use this

* Use modern for loops and correct final fields in common-util

* Remove some trivial rules

* exception - make field final

* serialisation - Remove unused code and fix incorrectly abstract class

* Rule incompatible with Jackson annotations

* type - Remove unused code and improve

* Rule not working correctly

* data - Fix field finals, verbose If

* cache - Fix field final

* operation - Fix throw exceptions and field finals

* jcs-cache-service - Use try with resources

* store - Remove unused code and make improvements

* store - Fix redundant Ifs

* graph - Fix throw exceptions and field finals

* integration-test - Fix duplications and Ifs

* integration-test - StackTrace supression

* hdfs-library - Use varargs

* spark-library - Final and varargs

* map-store - Finals and suppressions

* accumulo-store - Finals, simplify Ifs and suppressions

* spark-accumulo-library - Finals and exception stacks

* This AssignmentInOperand is probably ok

* time-library - Finals

* common-rest - Abstract class fixes and suppressions

* Do not run PMD with quick profile

* Revert some changes to fix compilation

* Supress classes which are not really abstract

* core-rest - Tidy and suppress some warnings

* spring-rest - Fix final and suppress a warnings

* proxy-store - Fix exception and FQN

* federated-store - Fix exceptions, finals and minor changes

* road-traffic - Fix exceptions, file IO and loggers

* False positive supression

* Fix missing charset

* Checkstyle fixes and a FB Suppression

* Copyright

* Scanner charset change for Java 8

* Confusing logic was refactored wrongly

* Revert "Confusing logic was refactored wrongly"

This reverts commit c96d589.

* Fix NamedView merge and document logic for it

* Revert unintended changes

* Checkstyle

* Add CodeQL analysis check workflow

* Copyright and add/improve action cache

* Speed up build for CodeQL action

* Apply suggestions from code review

Null swaps

Co-authored-by: lb324567 <108922572+lb324567@users.noreply.github.com>

* Fix suggestion typo

* RFileReaderIterator.java - Pass full exception instead of using string formatting of message

* Consistant use of println for main function user output

* Checkstyle

* Newline consistency

* Remove use of deprecated IOUtils method

* Revert change to add stack trace

* Apply suggestions from code review

More null reversals

Co-authored-by: lb324567 <108922572+lb324567@users.noreply.github.com>
Co-authored-by: t11947 <53758970+t11947@users.noreply.github.com>

* Fix problem with added suggestions

* Fix PMD problems after merge

---------

Co-authored-by: lb324567 <108922572+lb324567@users.noreply.github.com>
Co-authored-by: t11947 <53758970+t11947@users.noreply.github.com>
@GCHQDeveloper314
Copy link
Member

Closed by #2871

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation GitHub Actions, CI/CD
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants