Implement UseOutputTypeCorrectly

Engine/Helper.cs

@@ -2085,6 +2085,11 @@ public object VisitConvertExpression(ConvertExpressionAst convAst)
         {
             if (convAst != null)
             {
+                if (convAst.Type.TypeName.GetReflectionType() != null)
+                {
+                    return convAst.Type.TypeName.GetReflectionType().FullName;
+                }
+
                 return convAst.Type.TypeName.FullName;
             }
 
@@ -2135,6 +2140,11 @@ public object VisitTypeConstraint(TypeConstraintAst typeAst)
         {
             if (typeAst != null)
             {
+                if (typeAst.TypeName.GetReflectionType() != null)
+                {
+                    return typeAst.TypeName.GetReflectionType().FullName;
+                }
+
                 return typeAst.TypeName.FullName;
             }
 
@@ -2160,6 +2170,11 @@ public object VisitTypeExpression(TypeExpressionAst typeExpressionAst)
         {
             if (typeExpressionAst != null)
             {
+                if (typeExpressionAst.TypeName.GetReflectionType() != null)
+                {
+                    return typeExpressionAst.TypeName.GetReflectionType().FullName;
+                }
+
                 return typeExpressionAst.TypeName.FullName;
             }
 

RuleDocumentation/UseOutputTypeCorrectly.md

@@ -0,0 +1,37 @@
+#UseOutputTypeCorrectly 
+**Severity Level: Information**
+
+
+##Description
+
+If a return type is declared, the cmdlet must return that type. If a type is returned, a return type must be declared.
+
+##How to Fix
+
+To fix a violation of this rule, please check the OuputType attribute lists and the types that are returned in your code. You can get more details by running “Get-Help about_Functions_OutputTypeAttribute” command in Windows PowerShell. 
+
+##Example
+
+##Example
+Wrong:
+
+	function Get-Foo
+	{
+            [CmdletBinding()]
+            [OutputType([String])]
+            Param(
+            )
+            return "4
+	}
+
+Correct:
+
+	function Get-Foo
+	{
+            [CmdletBinding()]
+            [OutputType([String])]
+            Param(
+            )
+
+            return "bar"
+	}

Rules/ScriptAnalyzerBuiltinRules.csproj

@@ -68,6 +68,7 @@
     <Compile Include="AvoidUsingPlainTextForPassword.cs" />
     <Compile Include="AvoidUsingWMIObjectCmdlet.cs" />
     <Compile Include="AvoidUsingWriteHost.cs" />
+    <Compile Include="UseOutputTypeCorrectly.cs" />
     <Compile Include="MissingModuleManifestField.cs" />
     <Compile Include="PossibleIncorrectComparisonWithNull.cs" />
     <Compile Include="ProvideCommentHelp.cs" />

Rules/Strings.resx

@@ -678,4 +678,16 @@
   <data name="AvoidUsingWMIObjectCmdletName" xml:space="preserve">
     <value>AvoidUsingWMIObjectCmdlet</value>
   </data>
+  <data name="UseOutputTypeCorrectlyCommonName" xml:space="preserve">
+    <value>Use OutputType Correctly</value>
+  </data>
+  <data name="UseOutputTypeCorrectlyDescription" xml:space="preserve">
+    <value>The return types of a cmdlet should be declared using the OutputType attribute.</value>
+  </data>
+  <data name="UseOutputTypeCorrectlyError" xml:space="preserve">
+    <value>The cmdlet '{0}' returns an object of type '{1}' but this type is not declared in the OutputType attribute.</value>
+  </data>
+  <data name="UseOutputTypeCorrectlyName" xml:space="preserve">
+    <value>UseOutputTypeCorrectly</value>
+  </data>
 </root>

Rules/UseOutputTypeCorrectly.cs

@@ -0,0 +1,179 @@
+//
+// Copyright (c) Microsoft Corporation.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+//
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Management.Automation.Language;
+using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic;
+using System.ComponentModel.Composition;
+using System.Globalization;
+using System.Management.Automation;
+
+namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules
+{
+    /// <summary>
+    /// ProvideCommentHelp: Checks that objects return in a cmdlet have their types declared in OutputType Attribute
+    /// </summary>
+    [Export(typeof(IScriptRule))]
+    public class UseOutputTypeCorrectly : SkipTypeDefinition, IScriptRule
+    {
+        private IEnumerable<TypeDefinitionAst> _classes;
+
+        /// <summary>
+        /// AnalyzeScript: Checks that objects return in a cmdlet have their types declared in OutputType Attribute
+        /// </summary>
+        /// <param name="ast">The script's ast</param>
+        /// <param name="fileName">The name of the script</param>
+        /// <returns>A List of diagnostic results of this rule</returns>
+        public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
+        {
+            if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage);
+
+            DiagnosticRecords.Clear();
+            this.fileName = fileName;
+
+            _classes = ast.FindAll(item => item is TypeDefinitionAst && ((item as TypeDefinitionAst).IsClass), true).Cast<TypeDefinitionAst>();
+
+            ast.Visit(this);
+
+            return DiagnosticRecords;
+        }
+
+        /// <summary>
+        /// Visit function and checks that it is a cmdlet. If yes, then checks that any object returns must have a type declared
+        /// in the output type (the only exception is if the type is object)
+        /// </summary>
+        /// <param name="funcAst"></param>
+        /// <returns></returns>
+        public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst funcAst)
+        {
+            if (funcAst == null || funcAst.Body == null || funcAst.Body.ParamBlock == null
+                || funcAst.Body.ParamBlock.Attributes == null || funcAst.Body.ParamBlock.Attributes.Count == 0
+                || !funcAst.Body.ParamBlock.Attributes.Any(attr => attr.TypeName.GetReflectionType() == typeof(CmdletBindingAttribute)))
+            {
+                return AstVisitAction.Continue;
+            }
+
+            HashSet<string> outputTypes = new HashSet<string>();
+
+            foreach (AttributeAst attrAst in funcAst.Body.ParamBlock.Attributes)
+            {
+                if (attrAst.TypeName != null && attrAst.TypeName.GetReflectionType() == typeof(OutputTypeAttribute)
+                    && attrAst.PositionalArguments != null)
+                {
+                    foreach (ExpressionAst expAst in attrAst.PositionalArguments)
+                    {
+                        if (expAst is StringConstantExpressionAst)
+                        {
+                            Type type = Type.GetType((expAst as StringConstantExpressionAst).Value);
+                            if (type != null)
+                            {
+                                outputTypes.Add(type.FullName);
+                            }
+                        }
+                        else
+                        {
+                            TypeExpressionAst typeAst = expAst as TypeExpressionAst;
+                            if (typeAst != null && typeAst.TypeName != null)
+                            {
+                                if (typeAst.TypeName.GetReflectionType() != null)
+                                {
+                                    outputTypes.Add(typeAst.TypeName.GetReflectionType().FullName);
+                                }
+                                else
+                                {
+                                    outputTypes.Add(typeAst.TypeName.FullName);
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+
+            List<Tuple<string, StatementAst>> returnTypes = FindPipelineOutput.OutputTypes(funcAst, _classes);
+
+            foreach (Tuple<string, StatementAst> returnType in returnTypes)
+            {
+                string typeName = returnType.Item1;
+
+                if (String.IsNullOrEmpty(typeName)
+                    || String.Equals(typeof(Unreached).FullName, typeName, StringComparison.OrdinalIgnoreCase)
+                    || String.Equals(typeof(Undetermined).FullName, typeName, StringComparison.OrdinalIgnoreCase)
+                    || String.Equals(typeof(object).FullName, typeName, StringComparison.OrdinalIgnoreCase)
+                    || outputTypes.Contains(typeName, StringComparer.OrdinalIgnoreCase))
+                {
+                    continue;
+                }
+                else
+                {
+                    DiagnosticRecords.Add(new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.UseOutputTypeCorrectlyError,
+                        funcAst.Name, typeName), returnType.Item2.Extent, GetName(), DiagnosticSeverity.Information, fileName));
+                }
+            }
+
+            return AstVisitAction.Continue;
+        }
+
+        /// <summary>
+        /// GetName: Retrieves the name of this rule.
+        /// </summary>
+        /// <returns>The name of this rule</returns>
+        public string GetName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.UseOutputTypeCorrectlyName);
+        }
+
+        /// <summary>
+        /// GetCommonName: Retrieves the common name of this rule.
+        /// </summary>
+        /// <returns>The common name of this rule</returns>
+        public string GetCommonName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.UseOutputTypeCorrectlyCommonName);
+        }
+
+        /// <summary>
+        /// GetDescription: Retrieves the description of this rule.
+        /// </summary>
+        /// <returns>The description of this rule</returns>
+        public string GetDescription()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.UseOutputTypeCorrectlyDescription);
+        }
+
+        /// <summary>
+        /// Method: Retrieves the type of the rule: builtin, managed or module.
+        /// </summary>
+        public SourceType GetSourceType()
+        {
+            return SourceType.Builtin;
+        }
+
+        /// <summary>
+        /// GetSeverity: Retrieves the severity of the rule: error, warning of information.
+        /// </summary>
+        /// <returns></returns>
+        public RuleSeverity GetSeverity()
+        {
+            return RuleSeverity.Information;
+        }
+
+        /// <summary>
+        /// Method: Retrieves the module/assembly name the rule is from.
+        /// </summary>
+        public string GetSourceName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
+        }
+    }
+}

Tests/Rules/BadCmdlet.ps1

@@ -7,6 +7,8 @@
                   ConfirmImpact='Medium')]
     [Alias()]
     [OutputType([String])]
+    [OutputType("System.Int32", ParameterSetName="ID")]
+
     Param
     (
         # Param1 help description
@@ -46,13 +48,22 @@
     }
     Process
     {
+        $a = 4.5
+
+        if ($true)
+        {
+            $a
+        }
+
+        return @{"hash"="true"}
     }
     End
     {
     }
 }
 
 # Provide comment help should not be raised here because this is not exported
+#Output type rule should also not be raised here as this is not a cmdlet
 function NoComment
 {
     Write-Verbose "No Comment"

Tests/Rules/GoodCmdlet.ps1

@@ -28,7 +28,9 @@ function Get-File
                   HelpUri = 'http://www.microsoft.com/',
                   ConfirmImpact='Medium')]
     [Alias()]
-    [OutputType([String])]
+    [OutputType([String], [System.Double], [Hashtable])]
+    [OutputType("System.Int32", ParameterSetName="ID")]
+
     Param
     (
         # Param1 help description
@@ -75,6 +77,15 @@ function Get-File
             Write-Verbose "Write Verbose"
             Get-Process
         }
+
+        $a = 4.5
+
+        if ($true)
+        {
+            $a
+        }
+
+        return @{"hash"="true"}
     }
     End
     {