Skip to content

[ZEPPELIN-6507] Return empty list from FileInterpreter.completion() - #5394

Merged
ParkGyeongTae merged 1 commit into
apache:masterfrom
dani1552:ZEPPELIN-6507
Aug 9, 2026
Merged

[ZEPPELIN-6507] Return empty list from FileInterpreter.completion()#5394
ParkGyeongTae merged 1 commit into
apache:masterfrom
dani1552:ZEPPELIN-6507

Conversation

@dani1552

@dani1552 dani1552 commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

What is this PR for?

FileInterpreter.completion() returns null rather than an empty list. Per Effective Java Item 54, methods that return a collection should never return null, since it forces every caller to add a special-case null check.

In practice this base implementation is not reached today: the only concrete subclass, HDFSFileInterpreter, overrides completion() with a real implementation. However, if a future FileInterpreter subclass omits that override, the null would propagate through the completion call chain, which does not perform null checks.

This PR changes the base implementation to return Collections.emptyList() and adds a unit test that pins the contract.

What type of PR is it?

Improvement

Todos

  • - Return Collections.emptyList() from FileInterpreter.completion()
  • - Add testCompletionReturnsEmptyListInsteadOfNull to FileInterpreterTest

What is the Jira issue?

How should this be tested?

  • mvn test -pl file -Dtest=FileInterpreterTest
  • TestFileInterpreter, the test double already present in FileInterpreterTest, does not override completion(), so the new test exercises the base implementation directly.
  • All 5 tests in FileInterpreterTest pass locally.

Screenshots (if appropriate)

N/A

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

@ParkGyeongTae ParkGyeongTae left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. This path is unreachable today since HDFSFileInterpreter overrides it, but the fix guards against a future subclass omitting the override and propagating null through an unchecked call chain. Test correctly targets the base implementation via TestFileInterpreter. CI green aside from the unrelated npm-audit failure.

@ParkGyeongTae
ParkGyeongTae merged commit 560a808 into apache:master Aug 9, 2026
17 of 18 checks passed
@ParkGyeongTae

Copy link
Copy Markdown
Member

Merged into master (560a808).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants