Convert various Mustachio errors to be exceptions. (#3897)

I think I just got this wrong when I first implemented these; each of these is non-fatal, and expected to be caught later, so they should all be exceptions. This was all caught by the `avoid_catching_errors` lint rule.
dart-lang · Oct 3, 2024 · 5df03dd · 5df03dd
1 parent b0328fd
commit 5df03dd
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 45 deletions.
diff --git a/lib/src/mustachio/renderer_base.dart b/lib/src/mustachio/renderer_base.dart
@@ -5,11 +5,15 @@
 // See the Mustachio README at tool/mustachio/README.md for high-level
 // documentation.
 
+/// @docImport 'package:source_span/source_span.dart';
+library;
+
 import 'dart:collection';
 import 'dart:convert' show htmlEscape;
 
 import 'package:analyzer/file_system/file_system.dart';
 import 'package:meta/meta.dart';
+
 import 'parser.dart';
 
 // TODO(devoncarew): See is we can make this synchronous.
@@ -119,7 +123,7 @@ class Template {
                   partialTemplates: {...partialTemplates});
               partialTemplates[partialFile] = partialTemplate;
             } on FileSystemException catch (e) {
-              throw MustachioResolutionError(span.message(
+              throw MustachioResolutionException(span.message(
                   'FileSystemException (${e.message}) when reading partial:'));
             }
           }
@@ -188,27 +192,26 @@ abstract class RendererBase<T extends Object?> {
       if (property != null) {
         try {
           return property.renderVariable(context, property, remainingNames);
-          // ignore: avoid_catching_errors
-        } on PartialMustachioResolutionError catch (e) {
-          // The error thrown by [Property.renderVariable] does not have all of
-          // the names required for a decent error. We throw a new error here.
-          throw MustachioResolutionError(node.keySpan.message(
+        } on PartialMustachioResolutionException catch (e) {
+          // The exception thrown by [Property.renderVariable] does not have all
+          // of the names required for a decent error. We throw a new error
+          // here.
+          throw MustachioResolutionException(node.keySpan.message(
               "Failed to resolve '${e.name}' on ${e.contextType} while "
               'resolving $remainingNames as a property chain on any types in '
               'the context chain: $contextChainString, after first resolving '
               "'$firstName' to a property on $T"));
         }
       }
-      // ignore: avoid_catching_errors
-    } on _MustachioResolutionErrorWithoutSpan catch (e) {
-      throw MustachioResolutionError(node.keySpan.message(e.message));
+    } on _MustachioResolutionExceptionWithoutSpan catch (e) {
+      throw MustachioResolutionException(node.keySpan.message(e.message));
     }
 
     final parent = this.parent;
     if (parent != null) {
       return parent.getFields(node);
     } else {
-      throw MustachioResolutionError(node.keySpan.message(
+      throw MustachioResolutionException(node.keySpan.message(
           "Failed to resolve '${names.first}' as a property on any types in "
           'the context chain: $contextChainString'));
     }
@@ -241,7 +244,7 @@ abstract class RendererBase<T extends Object?> {
     if (property == null) {
       final parent = this.parent;
       if (parent == null) {
-        throw MustachioResolutionError(node.keySpan.message(
+        throw MustachioResolutionException(node.keySpan.message(
             "Failed to resolve '$key' as a property on any types in the "
             'current context'));
       } else {
@@ -312,7 +315,7 @@ class SimpleRenderer extends RendererBase<Object?> {
   @override
   Property<Object>? getProperty(String key) {
     if (_invisibleGetters.contains(key)) {
-      throw MustachioResolutionError(_failedKeyVisibilityMessage(key));
+      throw MustachioResolutionException(_failedKeyVisibilityMessage(key));
     } else {
       return null;
     }
@@ -325,7 +328,8 @@ class SimpleRenderer extends RendererBase<Object?> {
     if (names.length == 1 && firstName == '.') {
       return context.toString();
     } else if (_invisibleGetters.contains(firstName)) {
-      throw MustachioResolutionError(_failedKeyVisibilityMessage(firstName));
+      throw MustachioResolutionException(
+          _failedKeyVisibilityMessage(firstName));
     } else if (parent != null) {
       return parent!.getFields(node);
     } else {
@@ -380,7 +384,7 @@ class Property<T extends Object?> {
     if (remainingNames.isEmpty) {
       return getValue(c).toString();
     } else {
-      throw MustachioResolutionError(
+      throw MustachioResolutionException(
           _simpleResolveErrorMessage(remainingNames, typeString));
     }
   }
@@ -391,43 +395,43 @@ class Property<T extends Object?> {
       "annotation's 'visibleTypes' list";
 }
 
-/// An error indicating that a renderer failed to resolve a key.
-class MustachioResolutionError extends Error {
+/// An exception indicating that a renderer failed to resolve a key.
+class MustachioResolutionException implements Exception {
   final String message;
 
-  MustachioResolutionError(this.message);
+  MustachioResolutionException(this.message);
 
   @override
-  String toString() => 'MustachioResolutionError: $message';
+  String toString() => 'MustachioResolutionException: $message';
 }
 
-/// An error indicating that a renderer failed to resolve a follow-on name in a
-/// multi-name key.
-class PartialMustachioResolutionError extends Error {
+/// An exception indicating that a renderer failed to resolve a follow-on name
+/// in a multi-name key.
+class PartialMustachioResolutionException implements Exception {
   final String name;
 
   final Type contextType;
 
-  PartialMustachioResolutionError(this.name, this.contextType);
+  PartialMustachioResolutionException(this.name, this.contextType);
 }
 
-/// A Mustachio resolution error which is thrown in a position where the AST
-/// node is not known.
+/// A Mustachio resolution exception which is thrown in a position where the
+/// AST node is not known.
 ///
-/// This error should be caught and "re-thrown" as a [MustachioResolutionError]
-/// with a message derived from a [SourceSpan].
-class _MustachioResolutionErrorWithoutSpan extends Error {
+/// This exception should be caught and "re-thrown" as a
+/// [MustachioResolutionException] with a message derived from a [SourceSpan].
+class _MustachioResolutionExceptionWithoutSpan implements Exception {
   final String message;
 
-  _MustachioResolutionErrorWithoutSpan(this.message);
+  _MustachioResolutionExceptionWithoutSpan(this.message);
 }
 
 extension MapExtensions<T> on Map<String, Property<T>> {
   Property<T> getValue(String name) {
     if (containsKey(name)) {
       return this[name]!;
     } else {
-      throw PartialMustachioResolutionError(name, T);
+      throw PartialMustachioResolutionException(name, T);
     }
   }
 }
diff --git a/test/mustachio/aot_compiler_render_test.dart b/test/mustachio/aot_compiler_render_test.dart
@@ -554,7 +554,7 @@ void main() {
         ],
         'Foo()',
       ),
-      throwsA(const TypeMatcher<MustachioResolutionError>()
+      throwsA(const TypeMatcher<MustachioResolutionException>()
           .having((e) => e.message, 'message', contains('''
 line 1, column 8 of lib/templates/foo.html: Failed to resolve '[s2]' as a property on any types in the context chain: [Foo]
   ╷
@@ -573,7 +573,7 @@ line 1, column 8 of lib/templates/foo.html: Failed to resolve '[s2]' as a proper
         ],
         'Foo()',
       ),
-      throwsA(const TypeMatcher<MustachioResolutionError>()
+      throwsA(const TypeMatcher<MustachioResolutionException>()
           .having((e) => e.message, 'message', contains('''
 line 1, column 9 of lib/templates/foo.html: Failed to resolve '[s2]' as a property on any types in the context chain: [Foo]
   ╷
@@ -592,7 +592,7 @@ line 1, column 9 of lib/templates/foo.html: Failed to resolve '[s2]' as a proper
         ],
         'Bar()..foo = Foo()',
       ),
-      throwsA(const TypeMatcher<MustachioResolutionError>()
+      throwsA(const TypeMatcher<MustachioResolutionException>()
           .having((e) => e.message, 'message', contains('''
 line 1, column 8 of lib/templates/bar.html: Failed to resolve 'x' on Bar while resolving [x] as a property chain on any types in the context chain: context0.foo, after first resolving 'foo' to a property on Foo?
   ╷
@@ -612,7 +612,7 @@ line 1, column 8 of lib/templates/bar.html: Failed to resolve 'x' on Bar while r
         ],
         'Bar()..foo = Foo()',
       ),
-      throwsA(const TypeMatcher<MustachioResolutionError>()
+      throwsA(const TypeMatcher<MustachioResolutionException>()
           .having((e) => e.message, 'message', contains('''
 line 1, column 13 of lib/templates/bar.html: Failed to resolve '[x]' as a property on any types in the context chain: [Foo, Bar]
   ╷

diff --git a/test/mustachio/runtime_renderer_render_test.dart b/test/mustachio/runtime_renderer_render_test.dart
@@ -478,7 +478,7 @@ World
     var foo = Foo();
     expect(
         () => renderFoo(foo, fooTemplate),
-        throwsA(const TypeMatcher<MustachioResolutionError>()
+        throwsA(const TypeMatcher<MustachioResolutionException>()
             .having((e) => e.message, 'message', contains('''
 line 1, column 8 of ${fooTemplateFile.path}: Failed to resolve 's2' as a property on any types in the context chain: Foo
   ╷
@@ -494,7 +494,7 @@ line 1, column 8 of ${fooTemplateFile.path}: Failed to resolve 's2' as a propert
     var foo = Foo();
     expect(
         () => renderFoo(foo, fooTemplate),
-        throwsA(const TypeMatcher<MustachioResolutionError>()
+        throwsA(const TypeMatcher<MustachioResolutionException>()
             .having((e) => e.message, 'message', contains('''
 line 1, column 9 of ${fooTemplateFile.path}: Failed to resolve 's2' as a property on any types in the current context
   ╷
@@ -511,7 +511,7 @@ line 1, column 9 of ${fooTemplateFile.path}: Failed to resolve 's2' as a propert
     var bar = Bar()..foo = Foo();
     expect(
         () => renderBar(bar, barTemplate),
-        throwsA(const TypeMatcher<MustachioResolutionError>().having(
+        throwsA(const TypeMatcher<MustachioResolutionException>().having(
             (e) => e.message,
             'message',
             contains("Failed to resolve 'x' on Foo while resolving [x] as a "
@@ -527,7 +527,7 @@ line 1, column 9 of ${fooTemplateFile.path}: Failed to resolve 's2' as a propert
     var bar = Bar()..foo = Foo();
     expect(
         () => renderBar(bar, barTemplate),
-        throwsA(const TypeMatcher<MustachioResolutionError>().having(
+        throwsA(const TypeMatcher<MustachioResolutionException>().having(
             (e) => e.message,
             'message',
             contains("Failed to resolve 'x' as a property on any types in the "
@@ -542,7 +542,7 @@ line 1, column 9 of ${fooTemplateFile.path}: Failed to resolve 's2' as a propert
     var foo = Foo();
     expect(
         () => renderFoo(foo, fooTemplate),
-        throwsA(const TypeMatcher<MustachioResolutionError>().having(
+        throwsA(const TypeMatcher<MustachioResolutionException>().having(
             (e) => e.message,
             'message',
             contains('Failed to resolve [length] property chain on String'))));
@@ -556,7 +556,7 @@ line 1, column 9 of ${fooTemplateFile.path}: Failed to resolve 's2' as a propert
     var foo = Foo()..s1 = 'String';
     expect(
         () => renderFoo(foo, fooTemplate),
-        throwsA(const TypeMatcher<MustachioResolutionError>().having(
+        throwsA(const TypeMatcher<MustachioResolutionException>().having(
             (e) => e.message,
             'message',
             contains('[length] is a getter on String, which is not visible to '
@@ -571,7 +571,7 @@ line 1, column 9 of ${fooTemplateFile.path}: Failed to resolve 's2' as a propert
     var foo = Foo()..s1 = 'String';
     expect(
         () => renderFoo(foo, fooTemplate),
-        throwsA(const TypeMatcher<MustachioResolutionError>().having(
+        throwsA(const TypeMatcher<MustachioResolutionException>().having(
             (e) => e.message,
             'message',
             contains('[length] is a getter on String, which is not visible to '
@@ -591,7 +591,7 @@ line 1, column 9 of ${fooTemplateFile.path}: Failed to resolve 's2' as a propert
       ..writeAsStringSync('Text {{#foo}}{{>missing.mustache}}{{/foo}}');
     expect(
         () async => await Template.parse(barTemplateFile),
-        throwsA(const TypeMatcher<MustachioResolutionError>().having(
+        throwsA(const TypeMatcher<MustachioResolutionException>().having(
             (e) => e.message,
             'message',
             allOf(

diff --git a/tool/mustachio/codegen_aot_compiler.dart b/tool/mustachio/codegen_aot_compiler.dart
@@ -158,8 +158,7 @@ Future<Map<_AotCompiler, String>> _deduplicateRenderers(
     String compiledLubRenderer;
     try {
       compiledLubRenderer = await lubCompiler._compileToRenderer(referenceUris);
-      // ignore: avoid_catching_errors
-    } on MustachioResolutionError {
+    } on MustachioResolutionException {
       // Oops, switching to the LUB type prevents the renderer from compiling;
       // likely the properties accessed in the partial are not all declared on
       // the LUB type.
@@ -624,7 +623,7 @@ class _BlockCompiler {
     }
     if (getter == null) {
       var contextTypes = [for (var c in _contextStack) c.type];
-      throw MustachioResolutionError(node.keySpan
+      throw MustachioResolutionException(node.keySpan
           .message("Failed to resolve '$key' as a property on any types in the "
               'context chain: $contextTypes'));
     }
@@ -642,7 +641,7 @@ class _BlockCompiler {
     for (var secondaryKey in remainingNames) {
       getter = type.lookUpGetter2(secondaryKey, type.element.library);
       if (getter == null) {
-        throw MustachioResolutionError(node.keySpan.message(
+        throw MustachioResolutionException(node.keySpan.message(
             "Failed to resolve '$secondaryKey' on ${context.type} while "
             'resolving $remainingNames as a property chain on any types in '
             'the context chain: $contextChain, after first resolving '