Skip to content

[MAINTENANCE] Make return statements consistent to satisfy consistent-return - #2157

Open
stweil wants to merge 3 commits into
kitodo:mainfrom
stweil:fix/codacy-consistent-return
Open

stweil wants to merge 3 commits into
kitodo:mainfrom
stweil:fix/codacy-consistent-return

Conversation

@stweil

@stweil stweil commented Sep 20, 2026

Copy link
Copy Markdown
Member

Resolve the high-severity Codacy "consistent-return" findings by ensuring every code path of a function either always returns a value or never does:

  • DlfMediaPlayer/3rd-party/VideoFrame.js: keep the vendored code unchanged and silence the three unavoidable mixed-return spots with targeted eslint-disable-line comments.
  • DlfMediaPlayer/ThumbnailPreview.js: make onPointerMove() void and give mouseEventToPosition() explicit "return undefined;" guards.
  • DlfMediaPlayer/VariantGroups.js: return undefined when no active track.
  • SlubMediaPlayer/SlubMediaPlayer.js: stop returning the unused result of ui.handleEscape() and give prepareScreenshot() "return undefined;".
  • SlubMediaPlayer/.../RiaaEq.js: rewrite makeFilter() with if/else so the exhaustive NodeKey handling always returns.
  • SlubMediaPlayer/modals/BookmarkModal.js: add exhaustive switch defaults.
  • PageView/FulltextControl.js: return undefined when no segment matches.

Behavior is preserved (all changes make implicit undefined returns explicit). Verified with ESLint (consistent-return) and tsc --noEmit.

Assisted-by: OpenCode / qwen3.8-27b-thinking (Alibaba Cloud)

return this.makeMidFilter();
}

return this.makeTrebleFilter();

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@fschoelzel, would it be better to return undefined if none of the three cases matches?

@sebastian-meyer sebastian-meyer added the ⟳ maintenance A task to keep the code up-to-date and manageable. label Sep 21, 2026
@codecov

codecov Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (875b458) to head (1312669).
⚠️ Report is 54 commits behind head on main.

Additional details and impacted files
@@     Coverage Diff      @@
##   main   #2157   +/-   ##
============================
============================

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sebastian-meyer

Copy link
Copy Markdown
Member

I am not a fan of changing 3rd party libraries unless its functionally necessary, because it makes upgrading them more complicated.

I am interested in your opinion, @fschoelzel (and also your feedback on which files are unchanged 3rd party libraries and which are customized).

@stweil

stweil commented Sep 21, 2026 •

Copy link
Copy Markdown
Member Author

I am not a fan of changing 3rd party libraries unless its functionally necessary, because it makes upgrading them more complicated.

I agree, and ideally we should not use obviously unmaintained 3rd party libraries like https://github.com/allensarkisyan/VideoFrame. Either this library should be forked for Kitodo (then I can send a PR for the fork), or we need some alternative. The relevant code has only 271 lines.

@stweil

stweil commented Sep 21, 2026

Copy link
Copy Markdown
Member Author

The relevant code has only 271 lines.

These few lines in VideoFrame.js create 43 Codacy issues!

@stweil
stweil force-pushed the fix/codacy-consistent-return branch from 7d0e43f to 1312669 Compare September 27, 2026 06:58
@stweil

stweil commented Sep 27, 2026

Copy link
Copy Markdown
Member Author

Pull request #2161 replaces the complete 3rd party code which was changed here.

@stweil
stweil force-pushed the fix/codacy-consistent-return branch 2 times, most recently from 1312669 to 9a06380 Compare October 26, 2026 10:46
…-return

Resolve the high-severity Codacy "consistent-return" findings by ensuring
every code path of a function either always returns a value or never does:

- DlfMediaPlayer/ThumbnailPreview.js: make onPointerMove() void and give
  mouseEventToPosition() explicit "return undefined;" guards.
- DlfMediaPlayer/VariantGroups.js: return undefined when no active track.
- SlubMediaPlayer/SlubMediaPlayer.js: stop returning the unused result of
  ui.handleEscape() and give prepareScreenshot() "return undefined;".
- SlubMediaPlayer/.../RiaaEq.js: rewrite makeFilter() with if/else so the
  exhaustive NodeKey handling always returns.
- SlubMediaPlayer/modals/BookmarkModal.js: add exhaustive switch defaults.
- PageView/FulltextControl.js: return undefined when no segment matches.

Behavior is preserved (all changes make implicit undefined returns
explicit). Verified with ESLint (consistent-return) and tsc --noEmit.

Assisted-by: OpenCode / qwen3.8-27b-thinking (Alibaba Cloud)
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Codacy's ESLint runs consistent-return with treatUndefinedAsUnspecified,
where "return undefined" counts as a no-value return. The earlier
"if (track) { return ...; } return undefined;" form therefore triggered
"Method 'findActiveGroup' expected a return value".

Assign the group to a variable and return it unconditionally, which keeps
a single value-return (consistent under both rule semantics), preserves
the previous behavior (the group, or undefined), and type-checks (the
Record<GroupKey, Group> index stays a plain GroupKey).

Assisted-by: OpenCode / qwen3.8-27b-thinking (Alibaba Cloud)
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Replaces the object-based keyToGroup lookup with a Map so no dynamic
property access remains (Codacy detect-object-injection).

Assisted-by: OpenCode / qwen3.8-27b-thinking (Alibaba Cloud)
Signed-off-by: Stefan Weil <sw@weilnetz.de>
@stweil
stweil force-pushed the fix/codacy-consistent-return branch from 9a06380 to 1d76615 Compare October 26, 2026 10:50
@stweil

stweil commented Oct 26, 2026

Copy link
Copy Markdown
Member Author

I rebased the PR, fixed a Codacy issue which was marked as new although being old and removed the changes to 3rd party code which will be removed by PR #2161 anyway.

This branch has not been deployed

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

Labels

⟳ maintenance A task to keep the code up-to-date and manageable.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants