Skip to content

fix(web-app-files, web-client, web-runtime): fix sonar warning#13836

Open
mzner wants to merge 1 commit into
masterfrom
fix/sonar-warnings
Open

fix(web-app-files, web-client, web-runtime): fix sonar warning#13836
mzner wants to merge 1 commit into
masterfrom
fix/sonar-warnings

Conversation

@mzner

@mzner mzner commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Description

Related Issue

  • Fixes <issue_link>

Motivation and Context

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Screenshots (if appropriate):

Open tasks:

  • ...

@mzner mzner requested a review from LukasHirt June 2, 2026 11:43
@update-docs

update-docs Bot commented Jun 2, 2026

Copy link
Copy Markdown

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@kw-security

kw-security commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@mzner mzner force-pushed the fix/sonar-warnings branch 2 times, most recently from e78ecc6 to 942b8b2 Compare June 2, 2026 13:43
@mzner mzner force-pushed the fix/sonar-warnings branch from 942b8b2 to e4f253f Compare June 2, 2026 14:48
@sonarqubecloud

sonarqubecloud Bot commented Jun 2, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
40.1% Condition Coverage on New Code (required ≥ 50%)

See analysis details on SonarQube Cloud

@DeepDiver1975 DeepDiver1975 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.

Thanks for the SonarQube cleanup, @mzner. I reviewed the diff as a maintainer. The code changes themselves look correct and behaviourally safe, but there are a few blockers before this can be merged.

Code review — changes are sound

  • SharesNavigation.vue...(location.params || {})...location.params: spreading null/undefined into an object literal is a no-op in JS, so this is behaviourally identical. Good.
  • globalThis.location for window.location (web-app-files/index.ts, vault/index.ts, TopBar.vue, web-runtime/index.ts) — equivalent in the browser (globalThis === window). Note other window.location usages remain in the codebase (e.g. SaveAsModal.vue, buildUrl.ts, useFileActionsOpenShortcut.ts); fine since Sonar flags specific lines, just flagging the inconsistency for awareness.
  • node:path import prefix (dav.ts, parsers.ts) — explicit protocol, no behaviour change. Good.
  • md5sum ./* / sha256sum ./* in release-web.yml — guards against filenames being misread as flags; minor robustness win.
  • // NOSONAR comments (dav.ts property mapping, useFileActions.ts) — these only suppress, but the suppressed lines (null as <Type> type-mapping casts, the isVisible cognitive-complexity hint) are intentional and not real defects, so the suppressions are legitimate. No functional regression anywhere.

Blockers (please resolve)

  1. Merge conflict — the PR is currently in a conflicting state against master; please rebase.
  2. CI redSonarCloud Code Analysis (Quality Gate failed: 40.1% condition coverage on new code, needs ≥ 50%), check-tests-passed, and e2e-tests-part-3 are all failing. A bit ironic that a Sonar-cleanup PR trips the Sonar coverage gate — the new-code coverage condition needs attention (or the touched lines need coverage).
  3. Missing changelog item — this repo uses fragment-based changelog files under changelog/unreleased/, and the bot already requested one. Even for code-quality work please add a fragment per changelog/TEMPLATE (e.g. a Change:/Enhancement: entry referencing this PR), since the changelog gate is part of the project convention here.

Once the rebase is done, CI is green (including the Sonar coverage gate), and a changelog fragment is added, this is good to go. Holding off on approval until then.

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.

4 participants