Merge pull request #51 from jacehensley-wf/1.7.0/UIP-1590_deprecate_r…

…equired_annotation/dev

UIP-1590: Deprecate the Required annotation (Includes UIP-1835)

lib/src/component_declaration/annotations.dart

@@ -184,13 +184,48 @@ class StateMixin implements TypedMap {
   const StateMixin({this.keyNamespace: null});
 }
 
+/// Marks a `prop` as required to be set.
+///
+/// Validation occurs in `UiComponent.validateRequiredProps` which requires super calls into `componentWillMount` and
+/// `componentWillReceiveProps`.
+///
+///     @Props()
+///     abstract class FooProps {
+///       @requiredProp
+///       String requiredProp;
+///     }
+const Accessor requiredProp = const Accessor(isRequired: true);
+
+/// Marks a `prop` as required to be set, but allowed to be set explicitly to `null`.
+///
+/// Validation occurs in `UiComponent.validateRequiredProps` which requires super calls into `componentWillMount` and
+/// `componentWillReceiveProps`.
+///
+///     @Props()
+///     abstract class FooProps {
+///       @nullableRequiredProp
+///       String nullableRequiredProp;
+///     }
+const Accessor nullableRequiredProp = const Accessor(isRequired: true, isNullable: true);
+
 /// Annotation used with the `over_react` transformer to customize individual accessors (props/state fields).
 ///
+/// Validation occurs in `UiComponent.validateRequiredProps` which requires super calls into `componentWillMount` and
+/// `componentWillReceiveProps`.
+///
 ///     @Props()
 ///     abstract class FooProps {
 ///       @Accessor(keyNamespace: '', key: 'custom_key')
 ///       String bar;
+///
+///       @Accessor(isRequired: true)
+///       String requiredProp;
+///
+///       @Accessor(isRequired: true, isNullable: true)
+///       String nullableRequiredProp;
 ///     }
+///
+/// Related: [requiredProp], [nullableRequiredProp].
 class Accessor {
   /// A key for the annotated accessor, overriding the default of the accessor's name.
   final String key;
@@ -199,22 +234,28 @@ class Accessor {
   /// overriding the default of `'${enclosingClassName}.'`.
   final String keyNamespace;
 
+  /// Whether the accessor is required to be set.
+  final bool isRequired;
+
+  /// Whether setting a prop to null is allowed.
+  final bool isNullable;
+
+  /// The error message displayed when the accessor is not set.
+  final String requiredErrorMessage;
+
   const Accessor({
-    this.key: null,
-    this.keyNamespace: null
+    this.key,
+    this.keyNamespace,
+    this.isRequired = false,
+    this.isNullable = false,
+    this.requiredErrorMessage,
   });
 }
 
-/// Annotation used with the `over_react` transformer to express a specific prop is required to be set.
-///
-/// This is validated in `UiComponent.validateRequiredProps` which requires super calls into `componentWillMount` and
-/// `componentWillReceiveProps`.
+/// Deprecated.
 ///
-///     @Props()
-///     abstract class FooProps {
-///       @Required()
-///       String bar;
-///     }
+/// Use [Accessor], [requiredProp], or [nullableRequiredProp] instead.
+@Deprecated('2.0.0')
 class Required {
   /// Whether setting a prop to null is allowed.
   final bool isNullable;

lib/src/component_declaration/component_base.dart

@@ -14,6 +14,7 @@
 
 library over_react.component_declaration.component_base;
 
+import 'package:meta/meta.dart';
 import 'package:over_react/over_react.dart' show
     ClassNameBuilder,
     CssClassPropsMixin,
@@ -143,11 +144,13 @@ abstract class UiComponent<TProps extends UiProps> extends react.Component {
   }
 
   @override
+  @mustCallSuper
   void componentWillReceiveProps(Map newProps) {
     validateRequiredProps(newProps);
   }
 
   @override
+  @mustCallSuper
   void componentWillMount() {
     validateRequiredProps(props);
   }

lib/src/transformer/impl_generation.dart

@@ -399,13 +399,15 @@ class ImplGenerator {
 
             String accessorName = variable.name.name;
 
+            T getConstantAnnotation<T>(AnnotatedNode member, String name, T value) {
+              return member.metadata.any((annotation) => annotation.name?.name == name) ? value : null;
+            }
+
             annotations.Accessor accessorMeta = instantiateAnnotation(field, annotations.Accessor);
+            annotations.Accessor requiredProp = getConstantAnnotation(field, 'requiredProp', annotations.requiredProp);
+            annotations.Accessor nullableRequiredProp = getConstantAnnotation(field, 'nullableRequiredProp', annotations.nullableRequiredProp);
             annotations.Required requiredMeta = instantiateAnnotation(field, annotations.Required);
 
-            bool isRequired = requiredMeta != null;
-            bool isNullable = isRequired && requiredMeta.isNullable;
-            bool hasErrorMessage = isRequired && requiredMeta.message != null && requiredMeta.message.isNotEmpty;
-
             String individualKeyNamespace = accessorMeta?.keyNamespace ?? keyNamespace;
             String individualKey = accessorMeta?.key ?? accessorName;
 
@@ -415,11 +417,48 @@ class ImplGenerator {
             String constantName = '${generatedPrefix}prop__$accessorName';
             String constantValue = 'const $constConstructorName($keyConstantName';
 
-            if (isRequired) {
+            var annotationCount = 0;
+
+            if (accessorMeta != null) {
+              annotationCount++;
+
+              if (accessorMeta.isRequired) {
+                constantValue += ', isRequired: true';
+
+                if (accessorMeta.isNullable) constantValue += ', isNullable: true';
+
+                if (accessorMeta.requiredErrorMessage != null && accessorMeta.requiredErrorMessage.isNotEmpty) {
+                  constantValue += ', errorMessage: ${stringLiteral(accessorMeta.requiredErrorMessage)}';
+                }
+              }
+            }
+
+            if (requiredMeta != null) {
               constantValue += ', isRequired: true';
 
-              if (isNullable) constantValue += ', isNullable: true';
-              if (hasErrorMessage) constantValue += ', errorMessage: ${stringLiteral(requiredMeta.message)}';
+              if (requiredMeta.isNullable) constantValue += ', isNullable: true';
+
+              if (requiredMeta.message != null && requiredMeta.message.isNotEmpty) {
+                constantValue += ', errorMessage: ${stringLiteral(requiredMeta.message)}';
+              }
+            }
+
+            if (requiredProp != null) {
+              annotationCount++;
+              constantValue += ', isRequired: true';
+            }
+
+            if (nullableRequiredProp != null) {
+              annotationCount++;
+              constantValue += ', isRequired: true, isNullable: true';
+            }
+
+            if (annotationCount > 1) {
+              logger.error(
+                  '@requiredProp/@nullableProp/@Accessor cannot be used together.\n'
+                  'You can use `@Accessor(required: true)` or `isNullable: true` instead of the shorthand versions.',
+                  span: getSpan(sourceFile, field)
+              );
             }
 
             constantValue += ')';

pubspec.yaml

@@ -9,6 +9,7 @@ environment:
 dependencies:
   analyzer: ">=0.26.1+3 <0.30.0"
   barback: "^0.15.0"
+  meta: "^1.0.4"
   react: "^3.1.0"
   source_span: "^1.2.0"
   transformer_utils: "^0.1.1"

test/over_react/component_declaration/transformer_integration_tests/constant_required_accessor_integration_test.dart

@@ -0,0 +1,153 @@
+// Copyright 2016 Workiva Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+library over_react.component_declaration.transformer_integration_tests.constant_required_accessor_integration;
+
+import 'dart:html';
+
+import 'package:over_react/over_react.dart';
+import 'package:react/react_dom.dart' as react_dom;
+import 'package:test/test.dart';
+
+import '../../../test_util/test_util.dart';
+
+void main() {
+  group('properly identifies required props by', () {
+    group('throwing when a prop is required and not set', () {
+      test('on mount', () {
+        expect(() => render(ComponentTest()..nullable = true),
+            throwsPropError_Required('ComponentTestProps.required')
+        );
+      });
+
+      test('on re-render', () {
+        var mountNode = new DivElement();
+        react_dom.render((ComponentTest()
+          ..required = true
+          ..nullable = true
+        )(), mountNode);
+
+        expect(() => react_dom.render((ComponentTest()..nullable = true)(), mountNode),
+            throwsPropError_Required('ComponentTestProps.required')
+        );
+      });
+    });
+
+    group('throwing when a prop is required and set to null', () {
+      test('on mount', () {
+        expect(() => render(ComponentTest()
+          ..required = null
+          ..nullable = true
+        ), throwsPropError_Required('ComponentTestProps.required'));
+      });
+
+      test('on re-render', () {
+        var mountNode = new DivElement();
+        react_dom.render((ComponentTest()
+          ..required = true
+          ..nullable = true
+        )(), mountNode);
+
+        expect(
+            () => react_dom.render((ComponentTest()
+              ..required = null
+              ..nullable = true
+            )(), mountNode),
+            throwsPropError_Required('ComponentTestProps.required')
+        );
+      });
+    });
+
+    group('throwing when a prop is nullable and not set', () {
+      test('on mount', () {
+        expect(() => render(ComponentTest()..required = true),
+            throwsPropError_Required('ComponentTestProps.nullable'));
+      });
+
+      test('on re-render', () {
+        var mountNode = new DivElement();
+        react_dom.render((ComponentTest()
+          ..required = true
+          ..nullable = true
+        )(), mountNode);
+
+        expect(() => react_dom.render((ComponentTest()..required = true)(), mountNode),
+            throwsPropError_Required('ComponentTestProps.nullable')
+        );
+      });
+    });
+
+    group('not throwing when a prop is required and set', () {
+      test('on mount', () {
+        expect(() => render(ComponentTest()
+          ..nullable = true
+          ..required = true
+        ), returnsNormally);
+      });
+
+      test('on re-render', () {
+        var mountNode = new DivElement();
+        react_dom.render((ComponentTest()
+          ..required = true
+          ..nullable = true
+        )(), mountNode);
+
+        expect(() => react_dom.render((ComponentTest()
+          ..required = true
+          ..nullable = true
+        )(), mountNode), returnsNormally);
+      });
+    });
+
+    group('not throwing when a prop is nullable and set to null', () {
+      test('on mount', () {
+        expect(() => render(ComponentTest()
+          ..nullable = null
+          ..required = true
+        ), returnsNormally);
+      });
+
+      test('on re-render', () {
+        var mountNode = new DivElement();
+        react_dom.render((ComponentTest()
+          ..required = true
+          ..nullable = true
+        )(), mountNode);
+
+        expect(() => react_dom.render((ComponentTest()
+          ..required = true
+          ..nullable = null
+        )(), mountNode), returnsNormally);
+      });
+    });
+  });
+}
+
+@Factory()
+UiFactory<ComponentTestProps> ComponentTest;
+
+@Props()
+class ComponentTestProps extends UiProps {
+  @requiredProp
+  var required;
+
+  @nullableRequiredProp
+  var nullable;
+}
+
+@Component()
+class ComponentTestComponent extends UiComponent<ComponentTestProps> {
+  @override
+  render() => Dom.div()();
+}