Swap to SpotBugs

Question

Swap to SpotBugs

Closed this issue 10 months ago · 7 comments

Morf uses findbugs-maven-plugin which uses FindBugs 3.0.1.

FindBugs is a discontinued project, and SpotBugs is its successor. There's a Maven plugin and migration is straightforward.

I think Morf should swap to using SpotBugs.

Answer 1 · 2021-07-21T16:18:29.000Z

Having said this is straightforward, I've tried it out and discovered Morf was binding FindBugs to validate as oppsed to the default, verify.

Unfortunately, as FindBugs and SpotBugs both need access to the classes, in the current setup they run and find nothing to do as compile hasn't happened yet. Moving the plugin config to verify throws up a selection of errors for morf-core (the only module currently configured to run FindBugs).

Answer 2 · 2021-07-21T16:27:15.000Z

See https://github.com/wnicholson/morf/tree/switch_to_spotbugs

Answer 3 · 2022-01-05T13:13:11.000Z

I've ported that branch as-is over to alfasoftware so it runs in CI - see https://github.com/alfasoftware/morf/tree/switch_to_spotbugs

25 fails from the initial run in Travis:

Rule	Errors
NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE	21
NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE	2
NP_BOOLEAN_RETURN_NULL	1
NP_TOSTRING_COULD_RETURN_NULL	1

Output from the build, as far as morf-core:

[INFO] --- spotbugs-maven-plugin:4.3.0:check (sca-spotbugs) @ morf-core ---
[INFO] BugInstance size is 25
[INFO] Error size is 0
[INFO] Total bugs: 25
[ERROR] Medium: str must be non-null but is marked as nullable [org.alfasoftware.morf.dataset.FilteredDataSetProducerAdapter$1] At FilteredDataSetProducerAdapter.java:[line 60] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: table must be non-null but is marked as nullable [org.alfasoftware.morf.dataset.FilteredDataSetProducerAdapter$3$1] At FilteredDataSetProducerAdapter.java:[line 114] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: table must be non-null but is marked as nullable [org.alfasoftware.morf.dataset.FilteredDataSetProducerAdapter$3$2] At FilteredDataSetProducerAdapter.java:[line 129] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: input must be non-null but is marked as nullable [org.alfasoftware.morf.dataset.TableDataHomology$1] At TableDataHomology.java:[line 93] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: input must be non-null but is marked as nullable [org.alfasoftware.morf.dataset.TableDataHomology$2] At TableDataHomology.java:[line 157] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: input must be non-null but is marked as nullable [org.alfasoftware.morf.jdbc.DatabaseType$Registry$1] At DatabaseType.java:[lines 168-169] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: input must be non-null but is marked as nullable [org.alfasoftware.morf.jdbc.DatabaseType$Registry$2] At DatabaseType.java:[line 213] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: column must be non-null but is marked as nullable [org.alfasoftware.morf.metadata.SchemaUtils$1] At SchemaUtils.java:[line 769] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: value must be non-null but is marked as nullable [org.alfasoftware.morf.metadata.SchemaUtils$2] At SchemaUtils.java:[line 785] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: input must be non-null but is marked as nullable [org.alfasoftware.morf.metadata.SchemaValidator$2] At SchemaValidator.java:[line 125] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: input must be non-null but is marked as nullable [org.alfasoftware.morf.metadata.SchemaValidator$3] At SchemaValidator.java:[line 132] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: input must be non-null but is marked as nullable [org.alfasoftware.morf.metadata.SchemaValidator$4] At SchemaValidator.java:[line 350] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: input must be non-null but is marked as nullable [org.alfasoftware.morf.metadata.SchemaValidator$5] At SchemaValidator.java:[line 359] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: org.alfasoftware.morf.metadata.ValueConverters$NullValueConverter.booleanValue(Object) has Boolean return type and returns explicit null [org.alfasoftware.morf.metadata.ValueConverters$NullValueConverter] At ValueConverters.java:[line 388] NP_BOOLEAN_RETURN_NULL
[ERROR] Medium: org.alfasoftware.morf.sql.TempTransitionalBuilderWrapper$BuilderWrapper.toString() may return null [org.alfasoftware.morf.sql.TempTransitionalBuilderWrapper$BuilderWrapper, org.alfasoftware.morf.sql.TempTransitionalBuilderWrapper$BuilderWrapper] Returned at TempTransitionalBuilderWrapper.java:[line 57]Known null at TempTransitionalBuilderWrapper.java:[line 57] NP_TOSTRING_COULD_RETURN_NULL
[ERROR] Medium: column must be non-null but is marked as nullable [org.alfasoftware.morf.sql.element.SqlParameter$1] At SqlParameter.java:[line 81] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: input must be non-null but is marked as nullable [org.alfasoftware.morf.upgrade.ChangePrimaryKeyColumns$1] At ChangePrimaryKeyColumns.java:[line 97] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: step must be non-null but is marked as nullable [org.alfasoftware.morf.upgrade.UpgradePathFinder$1] At UpgradePathFinder.java:[line 165] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: view must be non-null but is marked as nullable [org.alfasoftware.morf.upgrade.ViewChanges$2] At ViewChanges.java:[line 349] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: table must be non-null but is marked as nullable [org.alfasoftware.morf.upgrade.adapt.FilteredSchema$1] At FilteredSchema.java:[lines 51-53] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: table must be non-null but is marked as nullable [org.alfasoftware.morf.upgrade.adapt.SelectiveSchema$1] At SelectiveSchema.java:[lines 48-50] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: table must be non-null but is marked as nullable [org.alfasoftware.morf.upgrade.adapt.TableOverrideSchema$1] At TableOverrideSchema.java:[line 51] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: builder must be non-null but is marked as nullable [org.alfasoftware.morf.util.Builder$Helper$1] At Builder.java:[line 50] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
[ERROR] Medium: Possible null pointer dereference in new org.alfasoftware.morf.xml.DirectoryDataSet(File) due to return value of called method [org.alfasoftware.morf.xml.DirectoryDataSet, org.alfasoftware.morf.xml.DirectoryDataSet] Dereferenced at DirectoryDataSet.java:[line 57]Known null at DirectoryDataSet.java:[line 57] NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE
[ERROR] Medium: Possible null pointer dereference in org.alfasoftware.morf.xml.DirectoryDataSet.clearDestination() due to return value of called method [org.alfasoftware.morf.xml.DirectoryDataSet, org.alfasoftware.morf.xml.DirectoryDataSet] Dereferenced at DirectoryDataSet.java:[line 70]Known null at DirectoryDataSet.java:[line 70] NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE

Answer 4 · 2023-08-16T11:46:54.000Z

FindBugs -> SpotBugs blocks a potential JDK upgrade.

#281 allowlists those failing rules so making progress with the upgrades unblocks the JDK upgrade.

It also upgrades Guice which similarly is needed for later build-time JDK compatibility, although strictly that change can be split out from the SpotBugs upgrade and done as part of a JDK upgrade.

Answer 5 · 2023-09-13T10:34:58.000Z

As noted, I've pulled the Guice upgrade back out of this, it can come later along with the JDK upgrade, leaving this just about SpotBugs.

Answer 6 · 2023-09-13T10:38:16.000Z

#283 proposes doing the swap, and allowlisting the failing rules (see above)

Answer 7 · 2023-11-23T15:50:27.000Z

#283 merged back in October.