Turn on warnings as errors. (#43)

Enable more warnings and fix ones that were easy.

Directory.Build.props

@@ -28,8 +28,7 @@
     <!-- Default to not creating a NuPkg. Set this to True in projects that should generate a NuPkg -->
     <IsPackable>False</IsPackable>
 
-    <!-- TODO - temporarily turn off warnings as errors while we get the build working on GitHub -->
-    <TreatWarningsAsErrors>false</TreatWarningsAsErrors>
+    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
 
     <!-- TODO - temporarily turn off warnings about XML comments while we get the build working on GitHub -->
     <NoWarn>1591</NoWarn>

LottieGen/Glob.cs

@@ -12,6 +12,8 @@ static class Glob
     /// <summary>
     /// Returns the files that match the given pattern.
     /// </summary>
+    /// <returns>A list of pairs identifying the path to each file, and a path relative to the deepest
+    /// non-wildcarded part of the input.</returns>
     internal static IEnumerable<(string path, string relativePath)> EnumerateFiles(string value)
     {
         // Break the pattern into non-wildcarded directory and the pattern to match.

Toolkit.ruleset

@@ -89,28 +89,39 @@
     <Rule Id="SA1633" Action="None" />
     <Rule Id="SA1634" Action="None" />
     <Rule Id="SA1652" Action="None" />
+
     <!-- Rules that differ from what other Toolkit projects use -->
-    <!-- SA1308 - disallows m_, s_, and c_ field name prefixes. -->
+    <!-- Field 's_foo' should not begin with the prefix 's_' -->
     <Rule Id="SA1308" Action="None" />
-    <!-- SA1005: Single line comment must begin with a space. Conflicts with our pattern for commenting out code. -->
+
+    <!-- Single line comment must begin with a space. Conflicts with our pattern for commenting out code. -->
     <Rule Id="SA1005" Action="None" />
-    <!-- SA1623: Documentation for read/write properties needs to mention the setter. Conflicts with SA1624 for properties with implicit fields. -->
+
+    <!-- Static members should appear before non-static members -->
+    <Rule Id="SA1204" Action="None" />
+
+    <!-- Documentation for read/write properties needs to mention the setter. Conflicts with SA1624 for properties with implicit fields. -->
     <Rule Id="SA1623"  Action="None" />
 
+    <!-- Element 'Foo' should declare an access modifier -->
+    <Rule Id="SA1400" Action="None" />
+
     <!-- Temporarily disabled rules for Lottie as we integrate into the Windows Community Toolkit -->
     <!-- SA1008: Opening parenthesis must not be preceded by a space. Not make sense in , ( where space
         needs space followed -->
     <Rule Id="SA1008" Action="None" />
+
+    <!-- Closing parenthesis should not be preceded by a space. -->
     <Rule Id="SA1009" Action="None" />
+
     <!-- SA1024: Colon must be followed by a space. Not make sense in Colors definition.-->
     <Rule Id="SA1024" Action="None" />
+
     <!-- SA1102: Query clause must follow previous clause. Not make sense with comments.-->
     <Rule Id="SA1102" Action="None" />
-    <Rule Id="SA1204" Action="None" />
+
+    <!-- Static readonly fields should begin with upper-case letter -->
     <Rule Id="SA1311" Action="None" />
-    <Rule Id="SA1313" Action="None" />
-    <Rule Id="SA1400" Action="None" />
-    <Rule Id="SA1514" Action="None" />
-    <Rule Id="SA1615" Action="None" />
+
   </Rules>
 </RuleSet>

source/Lottie/Instantiator.cs

@@ -39,6 +39,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie
         /// Creates a new instance of <see cref="Windows.UI.Composition.Visual"/>
         /// described by the given <see cref="WinCompData.Visual"/>.
         /// </summary>
+        /// <returns>The <see cref="Windows.UI.Composition.Visual"/>.</returns>
         internal static Wc.Visual CreateVisual(Wc.Compositor compositor, Wd.Visual visual)
         {
             var converter = new Instantiator(compositor);

source/Lottie/LottieVisualSource.cs

@@ -39,13 +39,13 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie
         Uri _uriSource;
         ContentFactory _contentFactory;
 
+        /// <summary>
+        /// Gets the options for the <see cref="LottieVisualSource"/>.
+        /// </summary>
         // Optimize Lotties by default. Optimization takes a little longer but usually produces much
         // more efficient translations. The only reason someone would turn optimization off is if
         // the time to translate is too high, but in that case the Lottie is probably going to perform
         // so badly on the machine that it won't really be usable with our without optimization.
-        /// <summary>
-        /// Gets the options for the <see cref="LottieVisualSource"/>.
-        /// </summary>
         public static DependencyProperty OptionsProperty { get; } =
             RegisterDp(nameof(Options), LottieVisualOptions.Optimize);
 
@@ -99,6 +99,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie
         /// <summary>
         /// Called by XAML to convert a string to an <see cref="IAnimatedVisualSource"/>.
         /// </summary>
+        /// <returns>The <see cref="LottieVisualSource"/> for the given url.</returns>
         public static LottieVisualSource CreateFromString(string uri)
         {
             var uriUri = StringToUri(uri);
@@ -138,10 +139,10 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie
             return LoadAsync(sourceUri == null ? null : new Loader(this, sourceUri)).AsAsyncAction();
         }
 
-        // TODO: currently explicitly implemented interfaces are causing a problem with .NET Native. Make them implicit for now.
         /// <summary>
         /// Implements <see cref="IDynamicAnimatedVisualSource"/>.
         /// </summary>
+        // TODO: currently explicitly implemented interfaces are causing a problem with .NET Native. Make them implicit for now.
         public event TypedEventHandler<IDynamicAnimatedVisualSource, object> AnimatedVisualInvalidated
         {
             add
@@ -159,14 +160,14 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie
             }
         }
 
-        // TODO: currently explicitly implemented interfaces are causing a problem with .NET Native. Make them implicit for now.
-        //bool IAnimatedVisualSource.TryCreateAnimatedVisual(
         /// <summary>
         /// Implements <see cref="IAnimatedVisualSource"/>.
         /// </summary>
         /// <param name="compositor">The <see cref="Compositor"/> that can be used as a factory for the resulting <see cref="IAnimatedVisual"/>.</param>
         /// <param name="diagnostics">An optional object that may provide extra information about the result.</param>
         /// <returns>An <see cref="IAnimatedVisual"/>.</returns>
+        // TODO: currently explicitly implemented interfaces are causing a problem with .NET Native. Make them implicit for now.
+        //bool IAnimatedVisualSource.TryCreateAnimatedVisual(
         public IAnimatedVisual TryCreateAnimatedVisual(
             Compositor compositor,
             out object diagnostics)
@@ -305,7 +306,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie
             }
 
             // Asynchronously loads WinCompData from a Lottie file.
-            internal async Task<ContentFactory> LoadAsync(LottieVisualOptions Options)
+            internal async Task<ContentFactory> LoadAsync(LottieVisualOptions options)
             {
                 if (_uri == null && _storageFile == null)
                 {
@@ -315,10 +316,10 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie
 
                 LottieVisualDiagnostics diagnostics = null;
                 Stopwatch sw = null;
-                if (Options.HasFlag(LottieVisualOptions.IncludeDiagnostics))
+                if (options.HasFlag(LottieVisualOptions.IncludeDiagnostics))
                 {
                     sw = Stopwatch.StartNew();
-                    diagnostics = new LottieVisualDiagnostics { Options = Options };
+                    diagnostics = new LottieVisualDiagnostics { Options = options };
                 }
 
                 var result = new ContentFactory(diagnostics);

source/LottieData/Animatable.cs

@@ -92,12 +92,13 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.LottieData
         public bool IsAnimated => _keyFrames.Length > 1;
 
         /// <summary>
-        /// Returns true if this value is always equal to the given value.
+        /// Returns <c>true</c> if this value is always equal to the given value.
         /// </summary>
+        /// <returns><c>true</c> if this value is always equal to the given value.</returns>
         public bool AlwaysEquals(T value) => !IsAnimated && value.Equals(InitialValue);
 
-        // Not a great hash code because it ignore the KeyFrames, but quick.
         /// <inheritdoc/>
+        // Not a great hash code because it ignore the KeyFrames, but quick.
         public override int GetHashCode() => InitialValue.GetHashCode();
 
         /// <inheritdoc/>

source/LottieData/AssetCollection.cs

@@ -25,6 +25,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.LottieData
         /// <summary>
         /// Returns the <see cref="Asset"/> with the given id, or null if not found.
         /// </summary>
+        /// <returns>The <see cref="Asset"/> with the given id, or null if not found.</returns>
         public Asset GetAssetById(string id)
         {
             if (id == null)

source/LottieData/GradientStop.cs

@@ -44,6 +44,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.LottieData
         /// Returns the first gradient stop in a sequence that is a color. Opacity will always be
         /// set to 1.
         /// </summary>
+        /// <returns>The first gradient stop in the sequence.</returns>
         public static Color GetFirstColor(ReadOnlySpan<GradientStop> gradientStops)
         {
             foreach (var stop in gradientStops)

source/LottieData/HoldEasing.cs

@@ -24,15 +24,15 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.LottieData
         /// <inheritdoc/>
         public override EasingType Type => EasingType.Hold;
 
-        // All SetpEeasings are equivalent.
         /// <inheritdoc/>
+        // All HoldEasings are equivalent.
         public override int GetHashCode() => (int)Type;
 
         /// <inheritdoc/>
         public override bool Equals(object obj) => Equals(obj as HoldEasing);
 
-        // All LinearEasings are equivalent.
         /// <inheritdoc/>
+        // All HoldEasings are equivalent.
         public bool Equals(HoldEasing other) => other != null;
 
         /// <inheritdoc/>

source/LottieData/LayerCollection.cs

@@ -35,11 +35,14 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.LottieData
         /// Returns the <see cref="Layer"/>s in the <see cref="LayerCollection"/> in
         /// painting order.
         /// </summary>
+        /// <returns>The <see cref="Layer"/>s in painting order.</returns>
         public IEnumerable<Layer> GetLayersBottomToTop() => _layers;
 
         /// <summary>
         /// Returns the <see cref="Layer"/> with the given id, or null if no matching <see cref="Layer"/> is found.
         /// </summary>
+        /// <returns>The corresponding <see cref="Layer"/> or null if <paramref name="id"/> does not match
+        /// a single <see cref="Layer"/> in the collection.</returns>
         public Layer GetLayerById(int? id)
         {
             if (!id.HasValue)

source/LottieData/LinearEasing.cs

@@ -23,15 +23,15 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.LottieData
         /// <inheritdoc/>
         public override string ToString() => nameof(LinearEasing);
 
-        // All LinearEasings are equivalent.
         /// <inheritdoc/>
+        // All LinearEasings are equivalent.
         public override int GetHashCode() => (int)Type;
 
         /// <inheritdoc/>
         public override bool Equals(object obj) => Equals(obj as LinearEasing);
 
-        // All LinearEasings are equivalent.
         /// <inheritdoc/>
+        // All LinearEasings are equivalent.
         public bool Equals(LinearEasing other) => other != null;
     }
 }

source/LottieData/LottieCompositionValidator.cs

@@ -28,6 +28,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.LottieData
         /// Validates the given <see cref="LottieComposition"/> against all of the validation rules.
         /// Returns a list of validation issues, or an empty list if no issues were found.
         /// </summary>
+        /// <returns>A list of the issues discovered during validation.</returns>
         public static (string Code, string Description)[] Validate(LottieComposition lottieComposition)
         {
             if (lottieComposition == null)

source/LottieReader/Serialization/LottieCompositionReader.cs

@@ -81,8 +81,9 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.LottieData.Serialization
         }
 
         /// <summary>
-        /// Parses a Lottie file to create a <see cref="LottieData.LottieComposition"/>.
+        /// Parses a Lottie file to create a <see cref="LottieComposition"/>.
         /// </summary>
+        /// <returns>A <see cref="LottieComposition"/> read from the json stream.</returns>
         public static LottieComposition ReadLottieCompositionFromJsonStream(
             Stream stream,
             Options options,

source/LottieToWinComp/LottieToWinCompTranslator.cs

@@ -47,10 +47,11 @@ using TypeConstraint = Microsoft.Toolkit.Uwp.UI.Lottie.WinCompData.Expressions.T
 
 namespace Microsoft.Toolkit.Uwp.UI.Lottie.LottieToWinComp
 {
-    // See: https://helpx.adobe.com/pdf/after_effects_reference.pdf for the After Effects semantics.
     /// <summary>
     /// Translates a <see cref="LottieData.LottieComposition"/> to an equivalent <see cref="Visual"/>.
     /// </summary>
+    /// <remarks>See https://helpx.adobe.com/pdf/after_effects_reference.pdf"/> for the
+    /// After Effects semantics.</remarks>
 #if PUBLIC
     public
 #endif
@@ -124,8 +125,9 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.LottieToWinComp
         /// <param name="strictTranslation">If true, throw an exception if translation issues are found.</param>
         /// <param name="visual">The <see cref="Visual"/> that contains the translated Lottie.</param>
         /// <param name="translationIssues">A list of issues that were encountered during the translation.</param>
+        /// <returns><c>true</c> if the <see cref="LottieComposition"/> was translated.</returns>
         public static bool TryTranslateLottieComposition(
-            LottieData.LottieComposition lottieComposition,
+            LottieComposition lottieComposition,
             bool strictTranslation,
             out Visual visual,
             out (string Code, string Description)[] translationIssues) =>
@@ -144,6 +146,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.LottieToWinComp
         /// <param name="addCodegenDescriptions">Add descriptions to objects for comments on generated code.</param>
         /// <param name="visual">The <see cref="Visual"/> that contains the translated Lottie.</param>
         /// <param name="translationIssues">A list of issues that were encountered during the translation.</param>
+        /// <returns><c>true</c> if the <see cref="LottieComposition"/> was translated.</returns>
         public static bool TryTranslateLottieComposition(
             LottieComposition lottieComposition,
             bool strictTranslation,
@@ -169,8 +172,6 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.LottieToWinComp
             return true;
         }
 
-        internal Optimizer Optimizer => _lottieDataOptimizer;
-
         void Translate()
         {
             var context = new TranslationContext(_lc);

source/WinCompData/CodeGen/CSharpInstantiatorGenerator.cs

@@ -37,6 +37,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.WinCompData.CodeGen
         /// Returns the C# code for a factory that will instantiate the given <see cref="Visual"/> as a
         /// Windows.UI.Composition Visual.
         /// </summary>
+        /// <returns>The C# code.</returns>
         public static string CreateFactoryCode(
             string className,
             Visual rootVisual,
@@ -55,8 +56,8 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.WinCompData.CodeGen
             return generator.GenerateCode(className, width, height);
         }
 
-        // Called by the base class to write the start of the file (i.e. everything up to the body of the Instantiator class).
         /// <inheritdoc/>
+        // Called by the base class to write the start of the file (i.e. everything up to the body of the Instantiator class).
         protected override void WriteFileStart(CodeBuilder builder, CodeGenInfo info)
         {
             if (info.RequiresWin2d)
@@ -97,8 +98,8 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.WinCompData.CodeGen
             builder.OpenScope();
         }
 
-        // Called by the base class to write the end of the file (i.e. everything after the body of the Instantiator class).
         /// <inheritdoc/>
+        // Called by the base class to write the end of the file (i.e. everything after the body of the Instantiator class).
         protected override void WriteFileEnd(
             CodeBuilder builder,
             CodeGenInfo info)

source/WinCompData/CodeGen/CxInstantiatorGenerator.cs

@@ -88,8 +88,8 @@ public:
 }}";
         }
 
-        // Called by the base class to write the start of the file (i.e. everything up to the body of the Instantiator class).
         /// <inheritdoc/>
+        // Called by the base class to write the start of the file (i.e. everything up to the body of the Instantiator class).
         protected override void WriteFileStart(
             CodeBuilder builder,
             CodeGenInfo info)
@@ -144,8 +144,8 @@ public:
             builder.WriteLine("ComPtr<ID2D1Factory> _d2dFactory;");
         }
 
-        // Called by the base class to write the end of the file (i.e. everything after the body of the Instantiator class).
         /// <inheritdoc/>
+        // Called by the base class to write the end of the file (i.e. everything after the body of the Instantiator class).
         protected override void WriteFileEnd(
             CodeBuilder builder,
             CodeGenInfo info)

source/WinCompData/CodeGen/InstantiatorGeneratorBase.cs

@@ -133,6 +133,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.WinCompData.CodeGen
         /// as  C# and C++.
         /// Returns null on failure.
         /// </summary>
+        /// <returns>A name, or null.</returns>
         public static string TrySynthesizeClassName(string name)
         {
             if (string.IsNullOrWhiteSpace(name))
@@ -280,6 +281,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.WinCompData.CodeGen
         /// <summary>
         /// Call this to generate the code. Returns a string containing the generated code.
         /// </summary>
+        /// <returns>The code.</returns>
         protected string GenerateCode(
             string className,
             float width,
@@ -344,6 +346,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.WinCompData.CodeGen
         /// <summary>
         /// Returns the code to call the factory for the given object.
         /// </summary>
+        /// <returns>The code to call the factory for the given object.</returns>
         protected string CallFactoryFor(CanvasGeometry obj)
         {
             return CallFactoryFromFor(_currentObjectFactoryNode, obj);
@@ -352,8 +355,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.WinCompData.CodeGen
         // Returns the code to call the factory for the given node from the given node.
         string CallFactoryFromFor(ObjectData callerNode, ObjectData calleeNode)
         {
-            string result;
-            if (callerNode.CallFactoryFromForCache.TryGetValue(calleeNode, out result))
+            if (callerNode.CallFactoryFromForCache.TryGetValue(calleeNode, out string result))
             {
                 // Return the factory from the cache.
                 return result;

source/WinCompData/CodeGen/NodeNamer.cs

@@ -26,6 +26,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.WinCompData.CodeGen
         /// Takes a list of nodes and generates unique names for them. Returns a list of node + name pairs.
         /// The names are chosen to be descriptive and usable in code generation.
         /// </summary>
+        /// <returns>A lot of node + name pairs usable in code generation.</returns>
         public static IEnumerable<(TNode, string)> GenerateNodeNames(IEnumerable<TNode> nodes)
         {
             var nodesByTypeName = new Dictionary<string, List<TNode>>();

source/WinCompData/Expressions/CubicBezierFunction2.cs

@@ -135,8 +135,8 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.WinCompData.Expressions
             }
         }
 
-        // (1-t)^3P0 + 3(1-t)^2tP1 + 3(1-t)t^2P2 + t^3P3
         /// <inheritdoc/>
+        // (1-t)^3P0 + 3(1-t)^2tP1 + 3(1-t)t^2P2 + t^3P3
         protected override Expression Simplify()
         {
             var oneMinusT = Subtract(Scalar(1), _t);

source/WinCompData/Expressions/Expression.cs

@@ -115,11 +115,13 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.WinCompData.Expressions
         /// <summary>
         /// Returns an equivalent expression, simplified if possible.
         /// </summary>
+        /// <returns>An equivalent expression, simplified if possible.</returns>
         protected abstract Expression Simplify();
 
         /// <summary>
         /// Returns the expression as a string for use by WinComp animations.
         /// </summary>
+        /// <returns>The exression as a string suitable for use in the Composition expression animation APIs.</returns>
         protected abstract string CreateExpressionString();
 
         protected static string Parenthesize(Expression expression) =>

source/WinCompData/Expressions/TypeAssert.cs

@@ -24,8 +24,8 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.WinCompData.Expressions
         /// <inheritdoc/>
         protected override Expression Simplify() => this;
 
-        // There is no syntax for a type assert, so just return the child syntax.
         /// <inheritdoc/>
+        // There is no syntax for a type assert, so just return the child syntax.
         protected override string CreateExpressionString() => _child.ToString();
 
         /// <inheritdoc/>

source/WinCompData/Tools/ApiCompatibility.cs

@@ -22,6 +22,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.WinCompData.Tools
         /// <summary>
         /// Analyzes the given tree and returns information about its compatibility with a runtime.
         /// </summary>
+        /// <returns>An object with properties describing the compatibility requirements of the tree.</returns>
         public static ApiCompatibility Analyze(CompositionObject graphRoot)
         {
             // Always require CompostionGeometryClip - this ensures that we are never compatible with

source/WinCompData/Tools/Graph.cs

@@ -20,6 +20,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.WinCompData.Tools
         /// <summary>
         /// Returns the graph of nodes reachable from the given <see cref="CompositionObject"/> root.
         /// </summary>
+        /// <returns>A <see cref="Graph"/> from the given Composition tree.</returns>
         public static ObjectGraph<Node> FromCompositionObject(CompositionObject root, bool includeVertices)
             => ObjectGraph<Node>.FromCompositionObject(root, includeVertices);
 

source/WinCompData/Tools/ObjectGraph.cs

@@ -34,6 +34,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.WinCompData.Tools
         /// <summary>
         /// Returns the graph of nodes reachable from the given <see cref="CompositionObject"/> root.
         /// </summary>
+        /// <returns>A <see cref="Graph"/> for the given Composition tree.</returns>
         public static new ObjectGraph<T> FromCompositionObject(CompositionObject root, bool includeVertices)
         {
             var result = new ObjectGraph<T>(includeVertices);

source/WinCompData/Wui/Color.cs

@@ -77,6 +77,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Lottie.WinCompData.Wui
         /// <summary>
         /// Attempts to get the friendly name for this color.
         /// </summary>
+        /// <returns><c>true</c> if a friendly name was returned.</returns>
         public bool TryGetFriendlyName(out string name)
         {
             name = GetFriendlyName(A, R, G, B);