From a0013323f2ee113b867a28fe432dbe1ffbe24cac Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Mon, 13 Jul 2026 23:06:28 +0200 Subject: [PATCH] chore(ui): enforce icon consistency --- .github/workflows/ci.yml | 5 + pubspec.lock | 2 +- pubspec.yaml | 1 + scripts/check_icon_consistency.dart | 207 +++++++++++++++++++++++++ scripts/ci_checks.sh | 12 +- scripts/test_check_icon_consistency.py | 79 ++++++++++ 6 files changed, 304 insertions(+), 2 deletions(-) create mode 100644 scripts/check_icon_consistency.dart create mode 100755 scripts/test_check_icon_consistency.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 951f7ba3..76c957a6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,6 +55,7 @@ jobs: python3 scripts/check_update_packages_workflow.py python3 scripts/test_pubspec_version.py python3 scripts/test_clean_translations.py + python3 scripts/test_check_icon_consistency.py - name: Verify formatting run: | @@ -62,6 +63,10 @@ jobs: [ ! -d test ] || paths+=(test) find "${paths[@]}" -name "*.dart" ! -name "*.g.dart" ! -name "*.freezed.dart" -type f -print0 | xargs -0 -r dart format --output=none --set-exit-if-changed + + - name: Verify icon consistency + run: dart run scripts/check_icon_consistency.dart + - name: Analyze code run: dart run scripts/check_analyzer.dart diff --git a/pubspec.lock b/pubspec.lock index 99863ccb..35d3b570 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -18,7 +18,7 @@ packages: source: hosted version: "0.3.7" analyzer: - dependency: transitive + dependency: "direct dev" description: name: analyzer sha256: de7148ed2fcec579b19f122c1800933dfa028f6d9fd38a152b04b1516cec120b diff --git a/pubspec.yaml b/pubspec.yaml index f71b0ee0..b8f7e441 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -97,6 +97,7 @@ dev_dependencies: path_provider_platform_interface: ^2.1.0 plugin_platform_interface: ^2.1.0 freezed: ^3.2.5 + analyzer: 10.0.1 dependency_overrides: auto_updater_platform_interface: diff --git a/scripts/check_icon_consistency.dart b/scripts/check_icon_consistency.dart new file mode 100644 index 00000000..0f0b7a98 --- /dev/null +++ b/scripts/check_icon_consistency.dart @@ -0,0 +1,207 @@ +import 'dart:io'; + +import 'package:analyzer/dart/analysis/utilities.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/source/line_info.dart'; + +const _canonicalAppIconPath = 'lib/widgets/app_icon.dart'; +const _generatedSuffixes = ['.g.dart', '.freezed.dart', '.gen.dart']; +final _generatedHeader = RegExp( + r'^\s*///?\s*(?:auto-)?generated\b.*\bdo not (?:edit|modify)\b', + caseSensitive: false, + multiLine: true, +); + +void main(List arguments) { + final scriptDirectory = File.fromUri(Platform.script).absolute.parent; + late final Directory root; + if (arguments.isEmpty) { + root = scriptDirectory.parent; + } else if (arguments.length == 2 && arguments.first == '--root') { + root = Directory(arguments.last).absolute; + } else { + stderr.writeln('Usage: dart run scripts/check_icon_consistency.dart [--root ]'); + exitCode = 64; + return; + } + final libDirectory = Directory('${root.path}${Platform.pathSeparator}lib'); + if (!libDirectory.existsSync()) { + stderr.writeln('lib:1:1: lib directory not found'); + exitCode = 1; + return; + } + + final files = + libDirectory + .listSync(recursive: true, followLinks: false) + .whereType() + .where((file) => file.path.endsWith('.dart')) + .toList() + ..sort((a, b) => a.path.compareTo(b.path)); + + final failures = <_Failure>[]; + var scannedFileCount = 0; + for (final file in files) { + final relativePath = _relativePath(file.path, root.path); + final source = file.readAsStringSync(); + if (_isGenerated(relativePath, source)) continue; + + scannedFileCount++; + final parseResult = parseString(content: source, path: file.path, throwIfDiagnostics: false); + final unit = parseResult.unit; + unit.accept( + _IconConsistencyVisitor( + path: relativePath, + lineInfo: parseResult.lineInfo, + allowFlutterIcon: relativePath == _canonicalAppIconPath, + flutterIconPrefixes: _importPrefixes(unit, const { + 'package:flutter/cupertino.dart', + 'package:flutter/material.dart', + 'package:flutter/widgets.dart', + }), + materialPrefixes: _importPrefixes(unit, const {'package:flutter/material.dart'}), + symbolsPrefixes: _importPrefixes(unit, const { + 'package:material_symbols_icons/material_symbols_icons.dart', + 'package:material_symbols_icons/symbols.dart', + }), + failures: failures, + ), + ); + } + + failures.sort((a, b) { + final pathComparison = a.path.compareTo(b.path); + if (pathComparison != 0) return pathComparison; + final lineComparison = a.line.compareTo(b.line); + if (lineComparison != 0) return lineComparison; + final columnComparison = a.column.compareTo(b.column); + if (columnComparison != 0) return columnComparison; + return a.message.compareTo(b.message); + }); + + if (failures.isNotEmpty) { + for (final failure in failures) { + stderr.writeln(failure); + } + stderr.writeln('Icon consistency check failed with ${failures.length} violation(s).'); + exitCode = 1; + return; + } + + stdout.writeln('Icon consistency check passed ($scannedFileCount non-generated files scanned).'); +} + +bool _isGenerated(String relativePath, String source) { + final fileName = relativePath.split('/').last; + if (_generatedSuffixes.any(fileName.endsWith)) return true; + if (relativePath.contains('/generated/')) return true; + + final headerLength = source.length < 1024 ? source.length : 1024; + return _generatedHeader.hasMatch(source.substring(0, headerLength)); +} + +String _relativePath(String path, String rootPath) { + final normalizedPath = path.replaceAll(r'\', '/'); + final normalizedRoot = rootPath.replaceAll(r'\', '/'); + return normalizedPath.substring(normalizedRoot.length + 1); +} + +Set _importPrefixes(CompilationUnit unit, Set uris) { + final prefixes = {}; + for (final directive in unit.directives.whereType()) { + final prefix = directive.prefix; + if (prefix != null && uris.contains(directive.uri.stringValue)) { + prefixes.add(prefix.name); + } + } + return prefixes; +} + +class _IconConsistencyVisitor extends RecursiveAstVisitor { + _IconConsistencyVisitor({ + required this.path, + required this.lineInfo, + required this.allowFlutterIcon, + required this.flutterIconPrefixes, + required this.materialPrefixes, + required this.symbolsPrefixes, + required this.failures, + }); + + final String path; + final LineInfo lineInfo; + final bool allowFlutterIcon; + final Set flutterIconPrefixes; + final Set materialPrefixes; + final Set symbolsPrefixes; + final List<_Failure> failures; + + @override + void visitInstanceCreationExpression(InstanceCreationExpression node) { + if (!allowFlutterIcon && node.constructorName.type.name.lexeme == 'Icon') { + _report(node.constructorName.type, 'Flutter Icon construction is forbidden; use AppIcon instead'); + } + super.visitInstanceCreationExpression(node); + } + + @override + void visitConstructorReference(ConstructorReference node) { + if (!allowFlutterIcon && node.constructorName.type.name.lexeme == 'Icon') { + _report(node.constructorName.type, 'Flutter Icon constructor tear-offs are forbidden; use AppIcon instead'); + } + super.visitConstructorReference(node); + } + + @override + void visitPrefixedIdentifier(PrefixedIdentifier node) { + final prefix = node.prefix.name; + final member = node.identifier.name; + if (prefix == 'Icons') { + _report(node, 'Icons.$member is forbidden; use a rounded Symbols member'); + } else if (prefix == 'Symbols' && !member.endsWith('_rounded')) { + _report(node, 'Symbols.$member must use its _rounded counterpart'); + } else if (!allowFlutterIcon && prefix == 'Icon' && member == 'new') { + _report(node, 'Flutter Icon constructor tear-offs are forbidden; use AppIcon instead'); + } + super.visitPrefixedIdentifier(node); + } + + @override + void visitPropertyAccess(PropertyAccess node) { + final target = node.target; + if (target is PrefixedIdentifier) { + final importPrefix = target.prefix.name; + final typeName = target.identifier.name; + final member = node.propertyName.name; + if (typeName == 'Icons' && materialPrefixes.contains(importPrefix)) { + _report(node, '$importPrefix.Icons.$member is forbidden; use a rounded Symbols member'); + } else if (typeName == 'Symbols' && symbolsPrefixes.contains(importPrefix) && !member.endsWith('_rounded')) { + _report(node, '$importPrefix.Symbols.$member must use its _rounded counterpart'); + } else if (!allowFlutterIcon && + typeName == 'Icon' && + member == 'new' && + flutterIconPrefixes.contains(importPrefix)) { + _report(node, 'Flutter Icon constructor tear-offs are forbidden; use AppIcon instead'); + } + } + super.visitPropertyAccess(node); + } + + void _report(AstNode node, String message) { + final location = lineInfo.getLocation(node.offset); + failures.add(_Failure(path: path, line: location.lineNumber, column: location.columnNumber, message: message)); + } +} + +class _Failure { + const _Failure({required this.path, required this.line, required this.column, required this.message}); + + final String path; + final int line; + final int column; + final String message; + + @override + String toString() => '$path:$line:$column: $message'; +} diff --git a/scripts/ci_checks.sh b/scripts/ci_checks.sh index 46525b8d..5f1741b7 100755 --- a/scripts/ci_checks.sh +++ b/scripts/ci_checks.sh @@ -85,13 +85,23 @@ section "workflow and script guards" if python3 scripts/check_build_workflow.py && python3 scripts/check_update_packages_workflow.py && python3 scripts/test_pubspec_version.py && - python3 scripts/test_clean_translations.py; then + python3 scripts/test_clean_translations.py && + python3 scripts/test_check_icon_consistency.py; then ok "workflow and script guards passed" else fail "workflow or script guard failed" FAILED=1 fi +# 5. Icon consistency +section "icon consistency" +if dart run scripts/check_icon_consistency.dart; then + ok "production icons use AppIcon and rounded Symbols" +else + fail "icon consistency violations found" + FAILED=1 +fi + # 3. Native formatting section "native format" out="$(mktemp)" diff --git a/scripts/test_check_icon_consistency.py b/scripts/test_check_icon_consistency.py new file mode 100755 index 00000000..5b482114 --- /dev/null +++ b/scripts/test_check_icon_consistency.py @@ -0,0 +1,79 @@ +#!/usr/bin/env python3 + +import subprocess +import tempfile +import unittest +from pathlib import Path + + +ROOT = Path(__file__).resolve().parent.parent +CHECKER = ROOT / "scripts" / "check_icon_consistency.dart" + + +class IconConsistencyCheckerTest(unittest.TestCase): + def run_checker(self, sources: dict[str, str]) -> subprocess.CompletedProcess[str]: + with tempfile.TemporaryDirectory() as temporary_directory: + fixture_root = Path(temporary_directory) + for relative_path, source in sources.items(): + target = fixture_root / relative_path + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(source, encoding="utf-8") + + return subprocess.run( + ["dart", "run", str(CHECKER), "--root", str(fixture_root)], + cwd=ROOT, + check=False, + capture_output=True, + text=True, + ) + + def test_accepts_canonical_wrapper_and_qualified_rounded_symbols(self) -> None: + result = self.run_checker( + { + "lib/widgets/app_icon.dart": """ +import 'package:flutter/material.dart'; + +Widget buildIcon(IconData icon) => Icon(icon); +""", + "lib/example.dart": """ +import 'package:material_symbols_icons/symbols.dart' as ms; + +Object buildIcon() => AppIcon(ms.Symbols.add_rounded); +""", + "lib/ignored.g.dart": """ +Widget ignored(IconData icon) => Icon(icon); +""", + } + ) + + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("Icon consistency check passed", result.stdout) + + def test_rejects_qualified_legacy_symbols_and_constructor_tear_offs(self) -> None: + result = self.run_checker( + { + "lib/bad.dart": """ +import 'package:flutter/material.dart' as material; +import 'package:material_symbols_icons/symbols.dart' as ms; +import 'package:material_symbols_icons/material_symbols_icons.dart' as material_symbols; + +final values = [ + material.Icons.add, + ms.Symbols.add, + material_symbols.Symbols.add, + material.Icon.new, + Icon.new, +]; +""", + } + ) + + self.assertEqual(result.returncode, 1, result.stdout) + self.assertIn("material.Icons.add is forbidden", result.stderr) + self.assertIn("ms.Symbols.add must use its _rounded counterpart", result.stderr) + self.assertIn("material_symbols.Symbols.add must use its _rounded counterpart", result.stderr) + self.assertEqual(result.stderr.count("constructor tear-offs are forbidden"), 2, result.stderr) + + +if __name__ == "__main__": + unittest.main()