Some static code analysis results by gramian · Pull Request #1290 · ArcadeData/arcadedb

gramian · 2023-10-29T11:27:05Z

What does this PR do?

These changes are a selection of results from static code analysis with spotbugs. The following is reported:

I added notes to lines which could potentially cause a null pointer dereference.
I added a note to a line which passes a null as parameter which is probably not allowed.
I added a note to an existing TODO with a potential explanation why this is a problem.
I fixed a test which likely used the wrong asserts
I removed a conditional which is likely unnecessary

@lvca you don't need to merge this PR, but you can look through them and see if sca and I are correct in pointing these out.

Motivation

Finding potential bugs.

Additional Notes

Anything else we should know when reviewing?

Checklist

I have run the build using mvn clean package command
My unit tests cover both failure and success scenarios

Changes to ArcadeData#1290

lvca · 2023-10-31T04:27:39Z

engine/src/main/java/com/arcadedb/database/DocumentValidator.java

                if (!((EmbeddedDocument) item).getType().instanceOf(ofType))
                  throwValidationException(p,
-                      "has been declared as LIST of '" + ofType + "' but an embedded document of type '" + embType + "' is used. Value: " + fieldValue);
+                      "has been declared as LIST of '" + ofType + "' but an embedded document of type '" + embType + "' is used. Value: " + fieldValue); // TODO: potential null pointer dereference


Confirm, this could lead to a NPE

lvca · 2023-10-31T04:29:38Z

engine/src/main/java/com/arcadedb/query/sql/executor/CheckClusterTypeStep.java

      if (bucketName != null)
        bucketObj = db.getSchema().getBucketByName(bucketName);
-      else if (bucket.getBucketName() != null)
+      else if (bucket.getBucketName() != null) // TODO: potential null pointer dereference?


Confirm, this could lead to a NPE

lvca · 2023-10-31T04:32:37Z

engine/src/main/java/com/arcadedb/query/sql/executor/TraverseExecutionPlanner.java


      final FetchFromClusterExecutionStep step = new FetchFromClusterExecutionStep(bucketId, context, profilingEnabled);
-      // TODO: THIS SEEMS A BUG
+      // TODO: THIS SEEMS A BUG (maybe because if null is passed equals always returns false?)


This code is the same in OrientDB: the support for the order by was never implemented in this class

lvca · 2023-10-31T04:35:14Z

engine/src/main/java/com/arcadedb/query/sql/parser/AlterPropertyStatement.java


    if (typez == null)
-      throw new CommandExecutionException("Invalid type name or type not found: " + typez);
+      throw new CommandExecutionException("Invalid type name or type not found: " + typez); // TODO: typez is always null here!


lvca · 2023-10-31T04:36:01Z

engine/src/main/java/com/arcadedb/query/sql/parser/RebuildIndexStatement.java

      final int batchSize) {
    if (idx == null)
-      throw new CommandExecutionException("Index '" + idx.getName() + "' not found");
+      throw new CommandExecutionException("Index '" + idx.getName() + "' not found"); // TODO: Null pointer derefence?


lvca · 2023-10-31T04:37:13Z

engine/src/main/java/com/arcadedb/schema/DocumentType.java


    final List<TypeIndex> list = new ArrayList<>(indexesByProperties.values());

-    if (polymorphic)


lvca · 2023-10-31T04:37:56Z

engine/src/test/java/com/arcadedb/query/sql/functions/math/SQLFunctionSquareRootTest.java

        function.execute(null, null, null, new Object[]{-4}, null);
        final Object result = function.getResult();
-        assertEquals(result, null);
+        assertNull(result);


… to 2.20.1 [skip ci] Bumps [org.codehaus.mojo:versions-maven-plugin](https://github.com/mojohaus/versions) from 2.19.1 to 2.20.1. Release notes *Sourced from [org.codehaus.mojo:versions-maven-plugin's releases](https://github.com/mojohaus/versions/releases).* > 2.20.1 > ------ > > 🐛 Bug Fixes > ----------- > > * Fixed [#1313](https://redirect.github.com/mojohaus/versions/issues/1313): Do not show existing version as update ([#1315](https://redirect.github.com/mojohaus/versions/pull/1315)) [`@andrzejj0`](https://github.com/andrzejj0) > > 2.20.0 > ------ > > 🚀 New features and improvements > ------------------------------- > > * Allow filtering out pre releases in use-latest-versions ([#1283](https://redirect.github.com/mojohaus/versions/pull/1283)) [`@Artur`](https://github.com/Artur)- > * [#979](https://redirect.github.com/mojohaus/versions/issues/979): Output file is not overwritten by default ([#1279](https://redirect.github.com/mojohaus/versions/pull/1279)) [`@andrzejj0`](https://github.com/andrzejj0) > > 🐛 Bug Fixes > ----------- > > * Fixed a problem with dependency management filtering in the logged results ([#1298](https://redirect.github.com/mojohaus/versions/pull/1298)) [`@andrzejj0`](https://github.com/andrzejj0) > * Fixes [#1295](https://redirect.github.com/mojohaus/versions/issues/1295): getAllUpdates(boolean) should respect currentVersionRange ([#1297](https://redirect.github.com/mojohaus/versions/pull/1297)) [`@andrzejj0`](https://github.com/andrzejj0) > * Fixed [#1287](https://redirect.github.com/mojohaus/versions/issues/1287) - Versionless dependencies in dependencyManagement accepted by maven, but not bij resolve-ranges ([#1288](https://redirect.github.com/mojohaus/versions/pull/1288)) [`@maroschutte`](https://github.com/maroschutte) > * Artifact comparison should use semantic version comparison. ([#1281](https://redirect.github.com/mojohaus/versions/pull/1281)) [`@andrzejj0`](https://github.com/andrzejj0) > * Resolves [#1150](https://redirect.github.com/mojohaus/versions/issues/1150): Resolve multiple level properties (properties resolving to properties) ([#1276](https://redirect.github.com/mojohaus/versions/pull/1276)) [`@andrzejj0`](https://github.com/andrzejj0) > > 📝 Documentation updates > ----------------------- > > * Add more examples of ignoredVersions config parameter ([#1296](https://redirect.github.com/mojohaus/versions/pull/1296)) [`@mikkoi`](https://github.com/mikkoi) > * Fix broken href link in site ([#1294](https://redirect.github.com/mojohaus/versions/pull/1294)) [`@mikkoi`](https://github.com/mikkoi) > * Added remaining javadoc comments. ([#1293](https://redirect.github.com/mojohaus/versions/pull/1293)) [`@andrzejj0`](https://github.com/andrzejj0) > * Getting rid of javadoc warnings ([#1292](https://redirect.github.com/mojohaus/versions/pull/1292)) [`@andrzejj0`](https://github.com/andrzejj0) > > 👻 Maintenance > ------------- > > * ResolverAdapter: a thin adapter over Resolver ([#1301](https://redirect.github.com/mojohaus/versions/pull/1301)) [`@andrzejj0`](https://github.com/andrzejj0) > * Fixed a problem with dependency management filtering in the logged results ([#1298](https://redirect.github.com/mojohaus/versions/pull/1298)) [`@andrzejj0`](https://github.com/andrzejj0) > * Fix broken href link in site ([#1294](https://redirect.github.com/mojohaus/versions/pull/1294)) [`@mikkoi`](https://github.com/mikkoi) > * Added remaining javadoc comments. ([#1293](https://redirect.github.com/mojohaus/versions/pull/1293)) [`@andrzejj0`](https://github.com/andrzejj0) > * Getting rid of javadoc warnings ([#1292](https://redirect.github.com/mojohaus/versions/pull/1292)) [`@andrzejj0`](https://github.com/andrzejj0) > * Removed a redundant integration test ([#1280](https://redirect.github.com/mojohaus/versions/pull/1280)) [`@andrzejj0`](https://github.com/andrzejj0) > > 📦 Dependency updates > -------------------- > > * Bump org.apache.commons:commons-lang3 from 3.19.0 to 3.20.0 ([#1312](https://redirect.github.com/mojohaus/versions/pull/1312)) @[dependabot[bot]](https://github.com/apps/dependabot) > * Bump byteBuddyVersion from 1.18.0 to 1.18.1 ([#1311](https://redirect.github.com/mojohaus/versions/pull/1311)) @[dependabot[bot]](https://github.com/apps/dependabot) > * Bump org.codehaus.plexus:plexus-archiver from 4.10.3 to 4.10.4 ([#1307](https://redirect.github.com/mojohaus/versions/pull/1307)) @[dependabot[bot]](https://github.com/apps/dependabot) > * Bump byteBuddyVersion from 1.17.7 to 1.18.0 ([#1309](https://redirect.github.com/mojohaus/versions/pull/1309)) @[dependabot[bot]](https://github.com/apps/dependabot) > * Bump commons-codec:commons-codec from 1.19.0 to 1.20.0 ([#1303](https://redirect.github.com/mojohaus/versions/pull/1303)) @[dependabot[bot]](https://github.com/apps/dependabot) > * Bump commons-io:commons-io from 2.20.0 to 2.21.0 ([#1305](https://redirect.github.com/mojohaus/versions/pull/1305)) @[dependabot[bot]](https://github.com/apps/dependabot) > * Bump org.codehaus.plexus:plexus-i18n from 1.0.0 to 1.1.0 ([#1306](https://redirect.github.com/mojohaus/versions/pull/1306)) @[dependabot[bot]](https://github.com/apps/dependabot) > * Bump org.codehaus.plexus:plexus-interactivity-api from 1.4 to 1.5.1 ([#1308](https://redirect.github.com/mojohaus/versions/pull/1308)) @[dependabot[bot]](https://github.com/apps/dependabot) > * Bump org.apache.maven.plugin-testing:maven-plugin-testing-harness from 3.3.0 to 3.4.0 ([#1302](https://redirect.github.com/mojohaus/versions/pull/1302)) @[dependabot[bot]](https://github.com/apps/dependabot) > * Bump org.codehaus.plexus:plexus-archiver from 4.10.2 to 4.10.3 ([#1290](https://redirect.github.com/mojohaus/versions/pull/1290)) @[dependabot[bot]](https://github.com/apps/dependabot) > * Bump org.codehaus.mojo:mojo-parent from 93 to 94 ([#1285](https://redirect.github.com/mojohaus/versions/pull/1285)) @[dependabot[bot]](https://github.com/apps/dependabot) ... (truncated) Commits * [`b296a4f`](mojohaus/versions@b296a4f) [maven-release-plugin] prepare release 2.20.1 * [`b243939`](mojohaus/versions@b243939) Fixed [#1313](https://redirect.github.com/mojohaus/versions/issues/1313): Do not show existing version as update ([#1315](https://redirect.github.com/mojohaus/versions/issues/1315)) * [`773d0f3`](mojohaus/versions@773d0f3) [maven-release-plugin] prepare for next development iteration * [`2467d99`](mojohaus/versions@2467d99) [maven-release-plugin] prepare release 2.20.0 * [`4c240e7`](mojohaus/versions@4c240e7) Bump org.apache.commons:commons-lang3 from 3.19.0 to 3.20.0 * [`6d64537`](mojohaus/versions@6d64537) Bump byteBuddyVersion from 1.18.0 to 1.18.1 * [`7736ca6`](mojohaus/versions@7736ca6) Bump org.codehaus.plexus:plexus-archiver from 4.10.3 to 4.10.4 * [`37a5330`](mojohaus/versions@37a5330) Bump byteBuddyVersion from 1.17.7 to 1.18.0 * [`edeb5e7`](mojohaus/versions@edeb5e7) Bump commons-codec:commons-codec from 1.19.0 to 1.20.0 * [`88874e0`](mojohaus/versions@88874e0) Bump commons-io:commons-io from 2.20.0 to 2.21.0 * Additional commits viewable in [compare view](mojohaus/versions@2.19.1...2.20.1) [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility\_score?dependency-name=org.codehaus.mojo:versions-maven-plugin&package-manager=maven&previous-version=2.19.1&new-version=2.20.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

gramian and others added 11 commits October 29, 2023 12:07

note potential np deref

df91e30

note potential np deref

bf20442

add note to TODO

f77fc0a

note potential np deref

1c41644

note potential np deref

2870787

remove unnecessary conditional

81976b1

fix asserts in test

f1002ce

note potential npe

d874a61

note potential np deref

2fd9d09

fix typo in test

268fe8c

fix: from @gramian static code run PR

c7d9542

Changes to ArcadeData#1290

lvca self-requested a review October 31, 2023 04:48

lvca assigned gramian Oct 31, 2023

lvca added this to the 23.10.1 milestone Oct 31, 2023

lvca added the bug label Oct 31, 2023

lvca approved these changes Oct 31, 2023

View reviewed changes

mergify bot merged commit 696d7ee into ArcadeData:main Oct 31, 2023

lvca added the fixed label Nov 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

Some static code analysis results#1290

Some static code analysis results#1290
mergify[bot] merged 11 commits intoArcadeData:mainfrom
gramian:main

gramian commented Oct 29, 2023

Uh oh!

lvca Oct 31, 2023

Uh oh!

lvca Oct 31, 2023

Uh oh!

lvca Oct 31, 2023

Uh oh!

lvca Oct 31, 2023

Uh oh!

lvca Oct 31, 2023

Uh oh!

lvca Oct 31, 2023

Uh oh!

lvca Oct 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		final List<TypeIndex> list = new ArrayList<>(indexesByProperties.values());

		if (polymorphic)

Uh oh!

Comments

Conversation

gramian commented Oct 29, 2023

What does this PR do?

Motivation

Additional Notes

Checklist

Uh oh!

lvca Oct 31, 2023

Choose a reason for hiding this comment

Uh oh!

lvca Oct 31, 2023

Choose a reason for hiding this comment

Uh oh!

lvca Oct 31, 2023

Choose a reason for hiding this comment

Uh oh!

lvca Oct 31, 2023

Choose a reason for hiding this comment

Uh oh!

lvca Oct 31, 2023

Choose a reason for hiding this comment

Uh oh!

lvca Oct 31, 2023

Choose a reason for hiding this comment

Uh oh!

lvca Oct 31, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants