From 0445ff0a808f6c2dc7089d387f9485a7676d6a41 Mon Sep 17 00:00:00 2001 From: kaufco Date: Thu, 10 Oct 2024 16:20:35 +0000 Subject: [PATCH 1/2] Create rule S7125 --- rules/S7125/java/metadata.json | 25 +++++++++++++++++++ rules/S7125/java/rule.adoc | 44 ++++++++++++++++++++++++++++++++++ rules/S7125/metadata.json | 2 ++ 3 files changed, 71 insertions(+) create mode 100644 rules/S7125/java/metadata.json create mode 100644 rules/S7125/java/rule.adoc create mode 100644 rules/S7125/metadata.json diff --git a/rules/S7125/java/metadata.json b/rules/S7125/java/metadata.json new file mode 100644 index 00000000000..fafebb5fc26 --- /dev/null +++ b/rules/S7125/java/metadata.json @@ -0,0 +1,25 @@ +{ + "title": "FIXME", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + ], + "defaultSeverity": "Major", + "ruleSpecification": "RSPEC-7125", + "sqKey": "S7125", + "scope": "All", + "defaultQualityProfiles": ["Sonar way"], + "quickfix": "unknown", + "code": { + "impacts": { + "MAINTAINABILITY": "HIGH", + "RELIABILITY": "MEDIUM", + "SECURITY": "LOW" + }, + "attribute": "CONVENTIONAL" + } +} diff --git a/rules/S7125/java/rule.adoc b/rules/S7125/java/rule.adoc new file mode 100644 index 00000000000..4172043c9d3 --- /dev/null +++ b/rules/S7125/java/rule.adoc @@ -0,0 +1,44 @@ +FIXME: add a description + +// If you want to factorize the description uncomment the following line and create the file. +//include::../description.adoc[] + +== Why is this an issue? + +FIXME: remove the unused optional headers (that are commented out) + +//=== What is the potential impact? + +== How to fix it +//== How to fix it in FRAMEWORK NAME + +=== Code examples + +==== Noncompliant code example + +[source,java,diff-id=1,diff-type=noncompliant] +---- +FIXME +---- + +==== Compliant solution + +[source,java,diff-id=1,diff-type=compliant] +---- +FIXME +---- + +//=== How does this work? + +//=== Pitfalls + +//=== Going the extra mile + + +//== Resources +//=== Documentation +//=== Articles & blog posts +//=== Conference presentations +//=== Standards +//=== External coding guidelines +//=== Benchmarks diff --git a/rules/S7125/metadata.json b/rules/S7125/metadata.json new file mode 100644 index 00000000000..2c63c085104 --- /dev/null +++ b/rules/S7125/metadata.json @@ -0,0 +1,2 @@ +{ +} From d66e6407d5d294f81c9d6cd5a3148f6d24504d2f Mon Sep 17 00:00:00 2001 From: Marco Kaufmann Date: Fri, 11 Oct 2024 16:40:42 +0200 Subject: [PATCH 2/2] Create rule S7125 --- rules/S7125/java/metadata.json | 8 +-- rules/S7125/java/rule.adoc | 119 +++++++++++++++++++++++++++------ 2 files changed, 102 insertions(+), 25 deletions(-) diff --git a/rules/S7125/java/metadata.json b/rules/S7125/java/metadata.json index fafebb5fc26..e952c4564b4 100644 --- a/rules/S7125/java/metadata.json +++ b/rules/S7125/java/metadata.json @@ -1,5 +1,5 @@ { - "title": "FIXME", + "title": "Null values should not be used in non-nullable input positions", "type": "CODE_SMELL", "status": "ready", "remediation": { @@ -11,14 +11,12 @@ "defaultSeverity": "Major", "ruleSpecification": "RSPEC-7125", "sqKey": "S7125", - "scope": "All", + "scope": "Main", "defaultQualityProfiles": ["Sonar way"], "quickfix": "unknown", "code": { "impacts": { - "MAINTAINABILITY": "HIGH", - "RELIABILITY": "MEDIUM", - "SECURITY": "LOW" + "RELIABILITY": "HIGH" }, "attribute": "CONVENTIONAL" } diff --git a/rules/S7125/java/rule.adoc b/rules/S7125/java/rule.adoc index 4172043c9d3..3afe9d69ef9 100644 --- a/rules/S7125/java/rule.adoc +++ b/rules/S7125/java/rule.adoc @@ -1,44 +1,123 @@ -FIXME: add a description - -// If you want to factorize the description uncomment the following line and create the file. -//include::../description.adoc[] - == Why is this an issue? -FIXME: remove the unused optional headers (that are commented out) +Using `null` in a non-nullable input position (e.g., as the right-hand side of an assignment, a function call argument, or a return statement argument) can lead to a NullPointerException (NPE) at runtime. This occurs because the receiving code typically assumes the value is non-null and omits null checks. -//=== What is the potential impact? +Formally, non-nullable and nullable versions of a type are distinct, with different domains. +The domain of a non-nullable type is _D_, while the domain of a nullable type is _D ∪ null_, a superset of _D_. +Thus, a non-null value can be used wherever a nullable type is expected, but not vice versa. +The only reason it's allowed by the compiler is that null-safety is not a built-in Java language feature, and it's therefore handled via nullability annotations by external tools bypassing the regular typing system. == How to fix it -//== How to fix it in FRAMEWORK NAME + +Depending on the use-case, there are different strategies to fix this problem === Code examples +**1. Change the input position type from non-nullable to nullable:** This resolves the issue at the reported location but may propagate it elsewhere. Note: you should avoid declaring everything nullable; only do so where it aligns with your data and state models. Otherwise, consider the other approaches. + ==== Noncompliant code example [source,java,diff-id=1,diff-type=noncompliant] ---- -FIXME +@NonNull String title = null; ---- ==== Compliant solution [source,java,diff-id=1,diff-type=compliant] ---- -FIXME +String title = null; +---- + +==== Noncompliant code example + +[source,java,diff-id=2,diff-type=noncompliant] +---- +@NullMarked +class Collector { + void collectData(List entities) { + // ... + } +} + +void process() { + collector.collectData(null); +} ---- -//=== How does this work? +==== Compliant solution -//=== Pitfalls +[source,java,diff-id=2,diff-type=compliant] +---- +class Collector { + void collectData(List entities) { + // ... + } +} -//=== Going the extra mile +void process() { + collector.collectData(null); +} +---- +**2. Replace `null` with a Guard Element:** This is particularly effective for array and collection types, where `null` can easily be replaced with an empty array or collection instance. -//== Resources -//=== Documentation -//=== Articles & blog posts -//=== Conference presentations -//=== Standards -//=== External coding guidelines -//=== Benchmarks +==== Noncompliant code example + +[source,java,diff-id=3,diff-type=noncompliant] +---- +@NullMarked +class Collector { + void collectData(List entities) { + // ... + } +} + +void process() { + collector.collectData(null); +} +---- + +==== Compliant solution + +[source,java,diff-id=3,diff-type=compliant] +---- +@NullMarked +class Collector { + void collectData(List entities) { + // ... + } +} + +void process() { + collector.collectData(List.of()); +} +---- + +**3. Throw an Exception:** For unexpected or uninitialized values or unspecified behavior, throw an exception instead of returning `null`. This reports the issue at its origin, not somewhere else in the source code where the unexpected `null` value suddenly becomes a problem. This makes debugging easier. + +==== Noncompliant code example + +[source,java,diff-id=4,diff-type=noncompliant] +---- +@NonNull State getNextState() { + return switch (state) { + case State.PENDING -> State.PROCESSING; + case State.PROCESSING -> State.PENDING; + default -> null; + }; +} +---- + +==== Compliant solution + +[source,java,diff-id=4,diff-type=compliant] +---- +@NonNull State getNextState() { + return switch (state) { + case State.PENDING -> State.PROCESSING; + case State.PROCESSING -> State.PENDING; + default -> throw new IllegalStateException(); + }; +} +----