Skip to content

ci: lint QML files with qmllint - #10855

Open
Alwoch wants to merge 2 commits into
spesmilo:masterfrom
Alwoch:ci_qmllint
Open

ci: lint QML files with qmllint#10855
Alwoch wants to merge 2 commits into
spesmilo:masterfrom
Alwoch:ci_qmllint

Conversation

@Alwoch

@Alwoch Alwoch commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Closes #10802

This PR creates a qml-lint CI job that runs qmllint over all tracked *.qml files (qml gui and plugins). The project currently uses PyQt6 which doesn't ship qmllint, so in this PR qmllint is taken from the PySide6 wheels pinned in the workflow. PySide6-Addons is also needed for the QtMultimedia types used by QRScan.qml. It is a lint-only dependency so nothing in electrum imports it.

As anticipated in the issue, the default behaviour is not usable as a gate because a plain qmllint over the tree currently emits approximately 3400 warnings, nearly all from three structural sources that static analysis cannot see: context properties injected at runtime (qeapp.py), dynamically scoped root-window properties (constants/app in main.qml), and the org.electrum types being registered from Python. To address this:

  • contrib/qml_lint/run_qml_lint.py generates a minimal qmldir+qmltypes stub for org.electrum from the qmlRegisterType calls in qeapp.py (regenerated on every run, so it cannot drift) and passes it to qmllint via -I. With the module resolvable, a misspelled type name fails the lint instead of producing unresolved-import noise.
  • .qmllint.ini at the repo root disables six warning categories; each carries a comment with the measured warning count and what it would take to enable it. Everything else gates: syntax errors, unknown types, bad imports, and the 56 remaining default-enabled categories the tree already passes (duplicate bindings, writes to read-only properties, use-before-declaration, deprecations, ...).
$ python3 -m venv /tmp/qml-lint-venv && /tmp/qml-lint-venv/bin/pip install "PySide6-Essentials==6.11.1" "PySide6-Addons==6.11.1"
$ /tmp/qml-lint-venv/bin/python contrib/qml_lint/run_qml_lint.py
[... 96 info-level unused-import notices, which do not gate ...]
qmllint: checked 131 files (29 org.electrum stub types), exit code 0

# after changing one "Label" to "Labell" in About.qml:
$ /tmp/qml-lint-venv/bin/python contrib/qml_lint/run_qml_lint.py
Warning: electrum/gui/qml/components/About.qml:35:13: Labell was not found. Did you add all imports and dependencies? [import]
            Labell {
            ^^^^^^
Info: Did you mean "Label"?
qmllint: checked 131 files (29 org.electrum stub types), exit code 255

Known gaps:

Property-name typos and misspelled identifiers inside bindings are not caught yet because they live in the disabled MissingProperty/UnqualifiedAccess categories. The disabled categories are intended as a ratchet, not a permanent state; this PR is deliberately infra-only, and I plan to follow up in roughly this order:

  1. Fix the 32 sites of Qt-documented undefined behavior that Quick.LayoutsPositioning flags (raw width/height on items managed by a Layout, e.g. About.qml:87 -- the file the issue quotes), then enable the category.
  2. Modernize the 43 legacy PropertyChanges sites, then enable Quick.PropertyChangesParsed.
  3. Extend the stub generator to introspect the Python bridge classes (pyqtProperty/signals/slots) so the org.electrum types carry real property lists, then enable MissingProperty (catching property-name typos) and UnresolvedAlias (1 site).
  4. Longer term, and only if the maintainers approve: refactor Constants into a QML singleton and migrate the context properties toward singletons (the direction Qt recommends), which is what it would take to ever enable UnqualifiedAccess (~3000 warnings today: ~800 constants, ~790 context properties, ~260 app, rest delegate scope). Unlike the items above, this changes runtime code, so it needs discussion first.

The 96 unused-import notices the job prints are also a candidate for a small cleanup PR; they are info-level and do not gate. The one remaining disable, EqualityTypeCoercion, is intentional and permanent: its single hit relies on url/string coercion, and switching it to !== would change behavior.

Defaults I picked that are easy to change:

The linter is pinned to 6.11.1 (newest) while the android gui ships Qt 6.10.2. I can test and repin to 6.10.x if matching the runtime Qt is preferred. .qmllint.ini sits at the repo root so it also covers the plugin qml files; it could instead live under electrum/gui/qml/, at the cost of not covering plugins. And plugin qml files are in scope, since they currently pass clean.

Let me know if this is along the lines of what you had in mind @SomberNight .

@Alwoch
Alwoch marked this pull request as draft August 14, 2026 14:39
@Alwoch
Alwoch marked this pull request as ready for review August 17, 2026 09:08
@accumulator

accumulator commented Aug 17, 2026

Copy link
Copy Markdown
Member

@Alwoch thank you for this, it looks quite comprehensive.

Maybe we could for now mark the job/step as continue-on-error so it doesn't fail the tests, and incrementally enable the currently disabled categories in .qmllint.ini, so we have a list of improvements to work towards.

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.

could we use qmllint (or similar) as linter for the qml gui?

2 participants