Attribute.equals has unreachable null guards, and the second ignores the attribute names
- Dominant language
- Scala
- Stars
- 314
- Forks
- 187
- Avg merge
- 1d 21h
- Merged PRs (30d)
- 214
Description
### What happened?
`Attribute.equals` has two null guards that are unreachable, and the second one is wrong if it ever *were* reached.
```java
Attribute that = (Attribute) toCompare;
if (this.attributeName == null) {
return that.attributeName == null;
}
if (this.attributeType == null) {
return that.attributeType == null; // ignores the names entirely
}
return this.attributeName.equalsIgnoreCase(that.attributeName)
&& this.attributeType.equals(that.attributeType);
```
**They cannot be reached.** The only constructor `checkNotNull`s both fields:
```java
public Attribute(String attributeName, AttributeType attributeType) {
checkNotNull(attributeName);
checkNotNull(attributeType);
...
}
```
**The second guard is also a latent bug.** If `attributeType` were ever null — say a future constructor, a deserialization path, or reflection — then `equals` returns `that.attributeType == null` **without comparing the names at all**. Two attributes with different names and null types would compare equal, which breaks the `Schema` lookups and `Set` semantics built on this class.
The safe fix is to delete both guards, since the constructor already guarantees non-null. If the guards are meant to survive, the second must still compare names.
Found while assessing test coverage; these lines are among the file's uncovered residue. **Deliberately not pinned by a test** — a test would have to construct an instance the constructor forbids, and pinning the current second guard would cement the name-ignoring comparison.
Note the file is a `.java` file living under `src/main/scala/`, which is easy to miss when searching.
### How to reproduce?
Static:
1. `common/workflow-core/src/main/scala/org/apache/texera/amber/core/tuple/Attribute.java` — the constructor's two `checkNotNull` calls, then the two null guards in `equals`.
2. Coverage on `main` shows those guard lines and their bodies as never executed.
### Version/Branch
1.3.0-incubating-SNAPSHOT (main)
### Was this issue authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)
Contributor guide
Assessment
This issue has not been assessed yet.