Skip to content

Commit 2af9cb8

Browse files
author
Quoc Truong
committed
Merge pull request #69 from PowerShell/returntyperule
Implement UseOutputTypeCorrectly
2 parents dff7c8d + 4f9972e commit 2af9cb8

File tree

9 files changed

+332
-1
lines changed

9 files changed

+332
-1
lines changed

Engine/Helper.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2085,6 +2085,11 @@ public object VisitConvertExpression(ConvertExpressionAst convAst)
20852085
{
20862086
if (convAst != null)
20872087
{
2088+
if (convAst.Type.TypeName.GetReflectionType() != null)
2089+
{
2090+
return convAst.Type.TypeName.GetReflectionType().FullName;
2091+
}
2092+
20882093
return convAst.Type.TypeName.FullName;
20892094
}
20902095

@@ -2135,6 +2140,11 @@ public object VisitTypeConstraint(TypeConstraintAst typeAst)
21352140
{
21362141
if (typeAst != null)
21372142
{
2143+
if (typeAst.TypeName.GetReflectionType() != null)
2144+
{
2145+
return typeAst.TypeName.GetReflectionType().FullName;
2146+
}
2147+
21382148
return typeAst.TypeName.FullName;
21392149
}
21402150

@@ -2160,6 +2170,11 @@ public object VisitTypeExpression(TypeExpressionAst typeExpressionAst)
21602170
{
21612171
if (typeExpressionAst != null)
21622172
{
2173+
if (typeExpressionAst.TypeName.GetReflectionType() != null)
2174+
{
2175+
return typeExpressionAst.TypeName.GetReflectionType().FullName;
2176+
}
2177+
21632178
return typeExpressionAst.TypeName.FullName;
21642179
}
21652180

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
#UseOutputTypeCorrectly
2+
**Severity Level: Information**
3+
4+
5+
##Description
6+
7+
If a return type is declared, the cmdlet must return that type. If a type is returned, a return type must be declared.
8+
9+
##How to Fix
10+
11+
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.
12+
13+
##Example
14+
15+
##Example
16+
Wrong:
17+
18+
function Get-Foo
19+
{
20+
[CmdletBinding()]
21+
[OutputType([String])]
22+
Param(
23+
)
24+
return "4
25+
}
26+
27+
Correct:
28+
29+
function Get-Foo
30+
{
31+
[CmdletBinding()]
32+
[OutputType([String])]
33+
Param(
34+
)
35+
36+
return "bar"
37+
}

Rules/ScriptAnalyzerBuiltinRules.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
<Compile Include="AvoidUsingPlainTextForPassword.cs" />
6969
<Compile Include="AvoidUsingWMIObjectCmdlet.cs" />
7070
<Compile Include="AvoidUsingWriteHost.cs" />
71+
<Compile Include="UseOutputTypeCorrectly.cs" />
7172
<Compile Include="MissingModuleManifestField.cs" />
7273
<Compile Include="PossibleIncorrectComparisonWithNull.cs" />
7374
<Compile Include="ProvideCommentHelp.cs" />

Rules/Strings.Designer.cs

Lines changed: 36 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Rules/Strings.resx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -678,4 +678,16 @@
678678
<data name="AvoidUsingWMIObjectCmdletName" xml:space="preserve">
679679
<value>AvoidUsingWMIObjectCmdlet</value>
680680
</data>
681+
<data name="UseOutputTypeCorrectlyCommonName" xml:space="preserve">
682+
<value>Use OutputType Correctly</value>
683+
</data>
684+
<data name="UseOutputTypeCorrectlyDescription" xml:space="preserve">
685+
<value>The return types of a cmdlet should be declared using the OutputType attribute.</value>
686+
</data>
687+
<data name="UseOutputTypeCorrectlyError" xml:space="preserve">
688+
<value>The cmdlet '{0}' returns an object of type '{1}' but this type is not declared in the OutputType attribute.</value>
689+
</data>
690+
<data name="UseOutputTypeCorrectlyName" xml:space="preserve">
691+
<value>UseOutputTypeCorrectly</value>
692+
</data>
681693
</root>

Rules/UseOutputTypeCorrectly.cs

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
//
2+
// Copyright (c) Microsoft Corporation.
3+
//
4+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
5+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
6+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
7+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
8+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
9+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
10+
// THE SOFTWARE.
11+
//
12+
13+
using System;
14+
using System.Collections.Generic;
15+
using System.Linq;
16+
using System.Management.Automation.Language;
17+
using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic;
18+
using System.ComponentModel.Composition;
19+
using System.Globalization;
20+
using System.Management.Automation;
21+
22+
namespace Microsoft.Windows.Powershell.ScriptAnalyzer.BuiltinRules
23+
{
24+
/// <summary>
25+
/// ProvideCommentHelp: Checks that objects return in a cmdlet have their types declared in OutputType Attribute
26+
/// </summary>
27+
[Export(typeof(IScriptRule))]
28+
public class UseOutputTypeCorrectly : SkipTypeDefinition, IScriptRule
29+
{
30+
private IEnumerable<TypeDefinitionAst> _classes;
31+
32+
/// <summary>
33+
/// AnalyzeScript: Checks that objects return in a cmdlet have their types declared in OutputType Attribute
34+
/// </summary>
35+
/// <param name="ast">The script's ast</param>
36+
/// <param name="fileName">The name of the script</param>
37+
/// <returns>A List of diagnostic results of this rule</returns>
38+
public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
39+
{
40+
if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage);
41+
42+
DiagnosticRecords.Clear();
43+
this.fileName = fileName;
44+
45+
_classes = ast.FindAll(item => item is TypeDefinitionAst && ((item as TypeDefinitionAst).IsClass), true).Cast<TypeDefinitionAst>();
46+
47+
ast.Visit(this);
48+
49+
return DiagnosticRecords;
50+
}
51+
52+
/// <summary>
53+
/// Visit function and checks that it is a cmdlet. If yes, then checks that any object returns must have a type declared
54+
/// in the output type (the only exception is if the type is object)
55+
/// </summary>
56+
/// <param name="funcAst"></param>
57+
/// <returns></returns>
58+
public override AstVisitAction VisitFunctionDefinition(FunctionDefinitionAst funcAst)
59+
{
60+
if (funcAst == null || funcAst.Body == null || funcAst.Body.ParamBlock == null
61+
|| funcAst.Body.ParamBlock.Attributes == null || funcAst.Body.ParamBlock.Attributes.Count == 0
62+
|| !funcAst.Body.ParamBlock.Attributes.Any(attr => attr.TypeName.GetReflectionType() == typeof(CmdletBindingAttribute)))
63+
{
64+
return AstVisitAction.Continue;
65+
}
66+
67+
HashSet<string> outputTypes = new HashSet<string>();
68+
69+
foreach (AttributeAst attrAst in funcAst.Body.ParamBlock.Attributes)
70+
{
71+
if (attrAst.TypeName != null && attrAst.TypeName.GetReflectionType() == typeof(OutputTypeAttribute)
72+
&& attrAst.PositionalArguments != null)
73+
{
74+
foreach (ExpressionAst expAst in attrAst.PositionalArguments)
75+
{
76+
if (expAst is StringConstantExpressionAst)
77+
{
78+
Type type = Type.GetType((expAst as StringConstantExpressionAst).Value);
79+
if (type != null)
80+
{
81+
outputTypes.Add(type.FullName);
82+
}
83+
}
84+
else
85+
{
86+
TypeExpressionAst typeAst = expAst as TypeExpressionAst;
87+
if (typeAst != null && typeAst.TypeName != null)
88+
{
89+
if (typeAst.TypeName.GetReflectionType() != null)
90+
{
91+
outputTypes.Add(typeAst.TypeName.GetReflectionType().FullName);
92+
}
93+
else
94+
{
95+
outputTypes.Add(typeAst.TypeName.FullName);
96+
}
97+
}
98+
}
99+
}
100+
}
101+
}
102+
103+
List<Tuple<string, StatementAst>> returnTypes = FindPipelineOutput.OutputTypes(funcAst, _classes);
104+
105+
foreach (Tuple<string, StatementAst> returnType in returnTypes)
106+
{
107+
string typeName = returnType.Item1;
108+
109+
if (String.IsNullOrEmpty(typeName)
110+
|| String.Equals(typeof(Unreached).FullName, typeName, StringComparison.OrdinalIgnoreCase)
111+
|| String.Equals(typeof(Undetermined).FullName, typeName, StringComparison.OrdinalIgnoreCase)
112+
|| String.Equals(typeof(object).FullName, typeName, StringComparison.OrdinalIgnoreCase)
113+
|| outputTypes.Contains(typeName, StringComparer.OrdinalIgnoreCase))
114+
{
115+
continue;
116+
}
117+
else
118+
{
119+
DiagnosticRecords.Add(new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.UseOutputTypeCorrectlyError,
120+
funcAst.Name, typeName), returnType.Item2.Extent, GetName(), DiagnosticSeverity.Information, fileName));
121+
}
122+
}
123+
124+
return AstVisitAction.Continue;
125+
}
126+
127+
/// <summary>
128+
/// GetName: Retrieves the name of this rule.
129+
/// </summary>
130+
/// <returns>The name of this rule</returns>
131+
public string GetName()
132+
{
133+
return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.UseOutputTypeCorrectlyName);
134+
}
135+
136+
/// <summary>
137+
/// GetCommonName: Retrieves the common name of this rule.
138+
/// </summary>
139+
/// <returns>The common name of this rule</returns>
140+
public string GetCommonName()
141+
{
142+
return string.Format(CultureInfo.CurrentCulture, Strings.UseOutputTypeCorrectlyCommonName);
143+
}
144+
145+
/// <summary>
146+
/// GetDescription: Retrieves the description of this rule.
147+
/// </summary>
148+
/// <returns>The description of this rule</returns>
149+
public string GetDescription()
150+
{
151+
return string.Format(CultureInfo.CurrentCulture, Strings.UseOutputTypeCorrectlyDescription);
152+
}
153+
154+
/// <summary>
155+
/// Method: Retrieves the type of the rule: builtin, managed or module.
156+
/// </summary>
157+
public SourceType GetSourceType()
158+
{
159+
return SourceType.Builtin;
160+
}
161+
162+
/// <summary>
163+
/// GetSeverity: Retrieves the severity of the rule: error, warning of information.
164+
/// </summary>
165+
/// <returns></returns>
166+
public RuleSeverity GetSeverity()
167+
{
168+
return RuleSeverity.Information;
169+
}
170+
171+
/// <summary>
172+
/// Method: Retrieves the module/assembly name the rule is from.
173+
/// </summary>
174+
public string GetSourceName()
175+
{
176+
return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
177+
}
178+
}
179+
}

Tests/Rules/BadCmdlet.ps1

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
ConfirmImpact='Medium')]
88
[Alias()]
99
[OutputType([String])]
10+
[OutputType("System.Int32", ParameterSetName="ID")]
11+
1012
Param
1113
(
1214
# Param1 help description
@@ -46,13 +48,22 @@
4648
}
4749
Process
4850
{
51+
$a = 4.5
52+
53+
if ($true)
54+
{
55+
$a
56+
}
57+
58+
return @{"hash"="true"}
4959
}
5060
End
5161
{
5262
}
5363
}
5464

5565
# Provide comment help should not be raised here because this is not exported
66+
#Output type rule should also not be raised here as this is not a cmdlet
5667
function NoComment
5768
{
5869
Write-Verbose "No Comment"

Tests/Rules/GoodCmdlet.ps1

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ function Get-File
2828
HelpUri = 'http://www.microsoft.com/',
2929
ConfirmImpact='Medium')]
3030
[Alias()]
31-
[OutputType([String])]
31+
[OutputType([String], [System.Double], [Hashtable])]
32+
[OutputType("System.Int32", ParameterSetName="ID")]
33+
3234
Param
3335
(
3436
# Param1 help description
@@ -75,6 +77,15 @@ function Get-File
7577
Write-Verbose "Write Verbose"
7678
Get-Process
7779
}
80+
81+
$a = 4.5
82+
83+
if ($true)
84+
{
85+
$a
86+
}
87+
88+
return @{"hash"="true"}
7889
}
7990
End
8091
{

0 commit comments

Comments
 (0)