diff --git a/cw_custom_lints/lib/main.dart b/cw_custom_lints/lib/main.dart index 39e07ba30..e57e22525 100644 --- a/cw_custom_lints/lib/main.dart +++ b/cw_custom_lints/lib/main.dart @@ -1,6 +1,8 @@ import "package:analysis_server_plugin/plugin.dart"; import "package:analysis_server_plugin/registry.dart"; import "package:cw_custom_lints/http_force_proxy/http_force_proxy_rule.dart"; +import "package:cw_custom_lints/modal_top_bar_semantics/modal_top_bar_semantics_rule.dart"; +import "package:cw_custom_lints/modern_button_semantics/modern_button_semantics_rule.dart"; import "package:cw_custom_lints/print_verbose/print_verbose_fix.dart"; import "package:cw_custom_lints/print_verbose/print_verbose_rule.dart"; import "package:cw_custom_lints/restricted_imports/restricted_imports_rule.dart"; @@ -16,6 +18,8 @@ class CwCustomLintsPlugin extends Plugin { registry.registerWarningRule(PrintVerboseRule()); registry.registerWarningRule(RestrictedImportsRule()); registry.registerWarningRule(HttpForceProxyRule()); + registry.registerWarningRule(ModernButtonSemanticsRule()); + registry.registerWarningRule(ModalTopBarSemanticsRule()); registry.registerFixForRule(PrintVerboseRule.code, ReplaceWithPrintV.new); } diff --git a/cw_custom_lints/lib/modal_top_bar_semantics/modal_top_bar_semantics_rule.dart b/cw_custom_lints/lib/modal_top_bar_semantics/modal_top_bar_semantics_rule.dart new file mode 100644 index 000000000..69eccaffe --- /dev/null +++ b/cw_custom_lints/lib/modal_top_bar_semantics/modal_top_bar_semantics_rule.dart @@ -0,0 +1,69 @@ +import "package:analyzer/analysis_rule/analysis_rule.dart"; +import "package:analyzer/analysis_rule/rule_context.dart"; +import "package:analyzer/analysis_rule/rule_visitor_registry.dart"; +import "package:analyzer/dart/ast/ast.dart"; +import "package:analyzer/dart/ast/visitor.dart"; +import "package:analyzer/error/error.dart"; +import "package:cw_custom_lints/utils/widget_arguments.dart"; + +class ModalTopBarSemanticsRule extends AnalysisRule { + ModalTopBarSemanticsRule() + : super( + name: "require_modal_top_bar_semantics", + description: + "ModalTopBar icons need a semantic label, because the icon alone does not say whether it closes the modal, goes back or does something else.", + ); + + static const LintCode code = LintCode( + "require_modal_top_bar_semantics", + "ModalTopBar was given a {0} without a {1}, so screen readers have nothing to announce.", + correctionMessage: "Pass a localized {1} next to the {0}.", + severity: DiagnosticSeverity.WARNING, + ); + + static const _iconSemanticLabels = { + "leadingIcon": "leadingSemanticLabel", + "trailingIcon": "trailingSemanticLabel", + }; + + @override + LintCode get diagnosticCode => code; + + @override + void registerNodeProcessors(RuleVisitorRegistry registry, RuleContext context) => + registry.addInstanceCreationExpression(this, _Visitor(this)); +} + +class _Visitor extends SimpleAstVisitor { + _Visitor(this.rule); + + final AnalysisRule rule; + + @override + void visitInstanceCreationExpression(InstanceCreationExpression node) { + if (!isCakeWalletWidget(node.constructorName, "ModalTopBar")) { + return; + } + + for (final slot in ModalTopBarSemanticsRule._iconSemanticLabels.entries) { + _checkSlot(node.argumentList, iconName: slot.key, semanticLabelName: slot.value); + } + } + + void _checkSlot( + ArgumentList argumentList, { + required String iconName, + required String semanticLabelName, + }) { + final icon = namedArgument(argumentList, iconName); + if (icon == null || icon.argumentExpression is NullLiteral) { + return; + } + + if (!isMissingOrEmptyText(namedArgumentExpression(argumentList, semanticLabelName))) { + return; + } + + rule.reportAtToken(icon.name, arguments: [iconName, semanticLabelName]); + } +} diff --git a/cw_custom_lints/lib/modern_button_semantics/modern_button_semantics_rule.dart b/cw_custom_lints/lib/modern_button_semantics/modern_button_semantics_rule.dart new file mode 100644 index 000000000..7824fced4 --- /dev/null +++ b/cw_custom_lints/lib/modern_button_semantics/modern_button_semantics_rule.dart @@ -0,0 +1,56 @@ +import "package:analyzer/analysis_rule/analysis_rule.dart"; +import "package:analyzer/analysis_rule/rule_context.dart"; +import "package:analyzer/analysis_rule/rule_visitor_registry.dart"; +import "package:analyzer/dart/ast/ast.dart"; +import "package:analyzer/dart/ast/visitor.dart"; +import "package:analyzer/error/error.dart"; +import "package:cw_custom_lints/utils/widget_arguments.dart"; + +class ModernButtonSemanticsRule extends AnalysisRule { + ModernButtonSemanticsRule() + : super( + name: "require_modern_button_semantics", + description: + "ModernButton needs a semanticLabel when it has no visible label, otherwise screen readers announce an unnamed button.", + ); + + static const LintCode code = LintCode( + "require_modern_button_semantics", + "ModernButton has no visible label, so screen readers have nothing to announce.", + correctionMessage: "Pass a localized semanticLabel, or a non-empty label.", + severity: DiagnosticSeverity.WARNING, + ); + + @override + LintCode get diagnosticCode => code; + + @override + void registerNodeProcessors(RuleVisitorRegistry registry, RuleContext context) => + registry.addInstanceCreationExpression(this, _Visitor(this)); +} + +class _Visitor extends SimpleAstVisitor { + _Visitor(this.rule); + + final AnalysisRule rule; + + @override + void visitInstanceCreationExpression(InstanceCreationExpression node) { + final constructorName = node.constructorName; + if (!isCakeWalletWidget(constructorName, "ModernButton")) { + return; + } + + final argumentList = node.argumentList; + final hasLabel = !isMissingOrEmptyText(namedArgumentExpression(argumentList, "label")); + final hasSemanticLabel = !isMissingOrEmptyText( + namedArgumentExpression(argumentList, "semanticLabel"), + ); + + if (hasLabel || hasSemanticLabel) { + return; + } + + rule.reportAtNode(constructorName); + } +} diff --git a/cw_custom_lints/lib/no_bare_catch/no_bare_catch_rule.dart b/cw_custom_lints/lib/no_bare_catch/no_bare_catch_rule.dart new file mode 100644 index 000000000..611ca6bfe --- /dev/null +++ b/cw_custom_lints/lib/no_bare_catch/no_bare_catch_rule.dart @@ -0,0 +1,51 @@ +import "package:analyzer/analysis_rule/analysis_rule.dart"; +import "package:analyzer/analysis_rule/rule_context.dart"; +import "package:analyzer/analysis_rule/rule_visitor_registry.dart"; +import "package:analyzer/dart/ast/ast.dart"; +import "package:analyzer/dart/ast/visitor.dart"; +import "package:analyzer/error/error.dart"; + +class NoBareCatchRule extends AnalysisRule { + NoBareCatchRule() + : super( + name: "no_bare_catch", + description: "please specify the exception class to catch (on SomeException catch (e))", + ); + + static const LintCode code = LintCode( + "no_bare_catch", + "please specify the exception class to catch (on SomeException catch (e))", + severity: DiagnosticSeverity.WARNING, + ); + + @override + LintCode get diagnosticCode => code; + + @override + void registerNodeProcessors(RuleVisitorRegistry registry, RuleContext context) { + registry.addCatchClause(this, _Visitor(this)); + } +} + +class _Visitor extends SimpleAstVisitor { + _Visitor(this.rule); + + final AnalysisRule rule; + + @override + void visitCatchClause(CatchClause node) { + if (node.exceptionType != null) { + return; + } + + final tryStatement = node.parent; + if (tryStatement is TryStatement) { + final precedingClauses = tryStatement.catchClauses.takeWhile((clause) => clause != node); + if (precedingClauses.any((clause) => clause.exceptionType != null)) { + return; + } + } + + rule.reportAtNode(node); + } +} diff --git a/cw_custom_lints/lib/utils/widget_arguments.dart b/cw_custom_lints/lib/utils/widget_arguments.dart new file mode 100644 index 000000000..cd6bb22d3 --- /dev/null +++ b/cw_custom_lints/lib/utils/widget_arguments.dart @@ -0,0 +1,39 @@ +import "package:analyzer/dart/ast/ast.dart"; + +bool isCakeWalletWidget(ConstructorName constructorName, String className) { + final element = constructorName.type.element; + if (element == null || element.name != className) { + return false; + } + + final libraryUri = element.library?.uri; + if (libraryUri == null || libraryUri.scheme != "package") { + return false; + } + + final segments = libraryUri.pathSegments; + return segments.isNotEmpty && segments.first == "cake_wallet"; +} + +NamedArgument? namedArgument(ArgumentList argumentList, String name) { + for (final argument in argumentList.arguments) { + if (argument is NamedArgument && argument.name.lexeme == name) { + return argument; + } + } + + return null; +} + +Expression? namedArgumentExpression(ArgumentList argumentList, String name) => + namedArgument(argumentList, name)?.argumentExpression; + +// passed null or "" +// does NOT work if a runtime expression evaluates to an empty string or null +bool isMissingOrEmptyText(Expression? expression) { + if (expression == null || expression is NullLiteral) { + return true; + } + + return expression is StringLiteral && (expression.stringValue?.isEmpty ?? false); +} diff --git a/lib/new-ui/widgets/modern_button.dart b/lib/new-ui/widgets/modern_button.dart index bba456d35..748aea817 100644 --- a/lib/new-ui/widgets/modern_button.dart +++ b/lib/new-ui/widgets/modern_button.dart @@ -28,9 +28,7 @@ class ModernButton extends StatelessWidget { this.backgroundColor, this.label, this.semanticLabel}) - : svgPath = null, - assert(semanticLabel != null || (label != null && label != ""), - "ModernButton needs a semanticLabel when it has no visible label"); + : svgPath = null; const ModernButton.svg( {super.key, @@ -42,9 +40,7 @@ class ModernButton extends StatelessWidget { this.backgroundColor, this.label, this.semanticLabel}) - : icon = null, - assert(semanticLabel != null || (label != null && label != ""), - "ModernButton needs a semanticLabel when it has no visible label"); + : icon = null; @override Widget build(BuildContext context) { diff --git a/lib/new-ui/widgets/receive_page/receive_top_bar.dart b/lib/new-ui/widgets/receive_page/receive_top_bar.dart index 374c58d07..4d49b4dd9 100644 --- a/lib/new-ui/widgets/receive_page/receive_top_bar.dart +++ b/lib/new-ui/widgets/receive_page/receive_top_bar.dart @@ -16,12 +16,7 @@ class ModalTopBar extends StatelessWidget { this.leadingWidget, this.trailingWidget, this.leadingSemanticLabel, - this.trailingSemanticLabel}) - : assert(leadingIcon == null || (leadingSemanticLabel != null && leadingSemanticLabel != ""), - "leadingIcon requires a non-empty leadingSemanticLabel"), - assert( - trailingIcon == null || (trailingSemanticLabel != null && trailingSemanticLabel != ""), - "trailingIcon requires a non-empty trailingSemanticLabel") { + this.trailingSemanticLabel}) { if (leadingIcon != null && leadingWidget != null) { throw Exception("Cannot have both leadingIcon and leadingWidget"); }