forcedotcom / forcedotcom/code-analyzer

[BUG][code-analyzer] sfge: a null collection element passed to a typed parameter aborts the entry point (NOT_A_MATCH, re: #1195)

Open
#2,096 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
240
Forks
52
Avg merge
1d 23h
Merged PRs (30d)
5

Description

### Have you tried to resolve this issue yourself first?

- [x] I confirm I have gone through the above steps and still have an issue to report.

### Bug Description

**Engine:** `sfge` (Salesforce Graph Engine) · **Rule:** `ApexFlsViolation` (DevPreview) · **Selector:** `--rule-selector sfge`

#1195 was closed on 2025-09-04 after the original reporter said it no longer reproduced. It still occurs on `code-analyzer` 5.15.0 / sfge 0.24.0, at the same throw site (`TypeableUtil.java:211`).

**Root cause.** `MethodTypeMatchUtil.getMatchRank` calls both halves of the match contract back to back on the same pair:

```java
// MethodTypeMatchUtil.java:186-193
private static int getMatchRank(int rank, Typeable typeable, ParameterVertex methodParameter) {
if (typeable == null || !typeable.matchesParameterType(methodParameter)) {
return NOT_A_MATCH; // graceful: sentinel, no throw
}

rank += typeable.rankParameterMatch(methodParameter); // throws on the same condition
return rank;
}
```

Both defaults live on `Typeable` and consult the same `getTypes()` set, so they agree — until a subclass overrides one. Nine classes override `matchesParameterType`; none override `rankParameterMatch`. Three of those overrides are deliberately more permissive, and two are safe because they widen `getTypes()` to match:

| Class | Override says it matches | Also widens `getTypes()`? | Result |
|---|---|---|---|
| `ApexIdValue:55` | its type **or `String`** | yes (`:68` -> `[Id, String, Object]`) | consistent |
| `ApexStringValue:150` | its type **or `Id`** | yes (`:164` -> `[String, Id, Object]`) | consistent |
| `LiteralExpressionVertex.Null:319` | **everything** (`return true;`) | **no** — inherits `[NULL, Object]` | **throws for every parameter type except `Object`** |

`LiteralExpressionVertex.Null` is the only class that widens one half without the other, which is exactly the observed message (`parameterType = String, type = [NULL, Object]`).

### Output / Logs

```shell
UnexpectedException: Did not expect NOT_A_MATCH when ranking parameter match. parameterType = String, type = OrderedTreeSet{internalList=[NULL, Object]}
at com.salesforce.graph.ops.TypeableUtil.rankParameterMatch(TypeableUtil.java:211)
at com.salesforce.graph.vertex.Typeable.rankParameterMatch(Typeable.java:41)
at com.salesforce.graph.ops.MethodTypeMatchUtil.getMatchRank(MethodTypeMatchUtil.java:191)
at com.salesforce.graph.ops.MethodTypeMatchUtil.parameterTypesMatch(MethodTypeMatchUtil.java:136)
at com.salesforce.graph.ops.MethodUtil.getInvoked(MethodUtil.java:358)
```

### Steps To Reproduce

1. Create an empty SFDX project (`sfdx-project.json` with a single `force-app` package directory).
2. Add `force-app/main/default/classes/NullCollectionElementToTypedParam.cls` with the class shown below, plus a standard `NullCollectionElementToTypedParam.cls-meta.xml` (apiVersion 62.0).
3. Add `code-analyzer.yml`:
```yaml
engines:
sfge:
java_thread_timeout: 900000
java_thread_count: 4
```
4. Run:
```
sf code-analyzer run --rule-selector sfge --workspace . --config-file code-analyzer.yml
```
5. The run reports an `InternalExecutionError` for the entry point instead of analysing it. That entry point yields no `ApexFlsViolation` findings at all, and nothing in the summary indicates coverage was lost.

```apex
public with sharing class NullCollectionElementToTypedParam {
private static String label(String value) {
return value == null ? 'none' : value;
}
@AuraEnabled
public static void run() {
List items = new List{ null };
insert new Account(Name = label(items[0]));
}
}
```

Note: the **collection element is essential**. `String v = null; label(v);` does *not* crash, because `MethodTypeMatchUtil.getDeclarationTypeWhenAvailable` substitutes the local's *declaration* vertex (`String`), whose `matchesParameterType` is the well-behaved default. A collection element has no declaration vertex to substitute, so the null-literal typed vertex survives. `Map` with a null value behaves identically.

### Expected Behavior

Give `LiteralExpressionVertex.Null` a `getTypes()` consistent with its "matches everything" claim — the same pattern `ApexIdValue` and `ApexStringValue` already follow.

Alternatively, have `rankParameterMatch` return `NOT_A_MATCH` instead of throwing; both callers already branch on that sentinel (`MethodTypeMatchUtil.java:155`, `MethodUtil.java:777`). That is one line, but it changes overload resolution wherever a permissive override currently crashes, so it probably wants a test sweep rather than a blind merge.

### Operating System

macOS 26.5.2

### Salesforce CLI Version

@salesforce/cli/2.147.7 darwin-arm64 node-v24.5.0

### Code Analyzer Plugin (code-analyzer) Version

code-analyzer 5.15.0

### Node Version

v24.5.0

### Java Version

openjdk version "11.0.32" 2026-07-21

### Python Version

N/A

### Additional Context (Screenshots, Files, etc)

A `null` sitting in a collection is entirely routine Apex, and this matches #1195's title exactly — "when null-assigned variable is passed into method".

If a unit test is more convenient for regression purposes, the defect also isolates to two calls without going through path evaluation. Against a `LiteralExpressionVertex.Null` pulled from a built graph and `SyntheticTypedVertex.get("String")`, all three of these hold on `code-analyzer-sfge-engine@dev`:

```java
assertThat(nullLiteral.getTypes().contains("String"), equalTo(false)); // hierarchy excludes it
assertThat(nullLiteral.matchesParameterType(stringParameter), equalTo(true)); // guard passes
assertThrows(UnexpectedException.class,
() -> nullLiteral.rankParameterMatch(stringParameter)); // next line throws
```

Happy to supply the full test file.

### Workaround

Avoid `null` entries in collections that flow into typed method parameters, or seed the collection with a non-null placeholder. Not realistic in general.

### Urgency

Moderate

Contributor guide

Open the contributing guide

Research direction

Start with MethodTypeMatchUtil.getMatchRank and LiteralExpressionVertex.Null, then reproduce with the supplied SFDX fixture and sf code-analyzer command. Compare the matchesParameterType and rankParameterMatch paths and inspect existing tests around these classes. Done means the null collection element no longer aborts analysis and a regression test covers the case without losing entry-point findings.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
devtools
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.