From 86e4d9a18ea378f3fc9df0aa4a02248f56f24fc9 Mon Sep 17 00:00:00 2001 From: Mansheng Yang Date: Sun, 5 Aug 2012 21:52:02 +0800 Subject: [PATCH] [CodeIssue] Added CastExpressionOfIncompatibleTypeIssue --- .../ICSharpCode.NRefactory.CSharp.csproj | 2 + .../CastExpressionOfIncompatibleTypeIssue.cs | 75 ++++++++++ .../ExpressionIsAlwaysOfProvidedTypeIssue.cs | 3 +- .../ExpressionIsNeverOfProvidedTypeIssue.cs | 93 +------------ .../Refactoring/TypeCompatibilityHelper.cs | 128 ++++++++++++++++++ ...tExpressionOfIncompatibleTypeIssueTests.cs | 50 +++++++ .../ICSharpCode.NRefactory.Tests.csproj | 10 +- 7 files changed, 262 insertions(+), 99 deletions(-) create mode 100644 ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/CastExpressionOfIncompatibleTypeIssue.cs create mode 100644 ICSharpCode.NRefactory.CSharp/Refactoring/TypeCompatibilityHelper.cs create mode 100644 ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/CastExpressionOfIncompatibleTypeIssueTests.cs diff --git a/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj b/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj index 64787b437d..e33a2add1c 100644 --- a/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj +++ b/ICSharpCode.NRefactory.CSharp/ICSharpCode.NRefactory.CSharp.csproj @@ -289,6 +289,7 @@ + @@ -326,6 +327,7 @@ + diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/CastExpressionOfIncompatibleTypeIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/CastExpressionOfIncompatibleTypeIssue.cs new file mode 100644 index 0000000000..ff2ad06ef5 --- /dev/null +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/CastExpressionOfIncompatibleTypeIssue.cs @@ -0,0 +1,75 @@ +// +// CastExpressionOfIncompatibleTypeIssue.cs +// +// Author: +// Mansheng Yang +// +// Copyright (c) 2012 Mansheng Yang +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// 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.Collections.Generic; +using ICSharpCode.NRefactory.TypeSystem; + +namespace ICSharpCode.NRefactory.CSharp.Refactoring +{ + [IssueDescription ("Type cast expression of incompatible type", + Description = "Type cast expression of incompatible type", + Category = IssueCategories.CodeQualityIssues, + Severity = Severity.Warning, + IssueMarker = IssueMarker.Underline)] + public class CastExpressionOfIncompatibleTypeIssue : ICodeIssueProvider + { + public IEnumerable GetIssues (BaseRefactoringContext context) + { + return new GatherVisitor (context).GetIssues (); + } + + class GatherVisitor : GatherVisitorBase + { + public GatherVisitor (BaseRefactoringContext ctx) + : base (ctx) + { + } + + public override void VisitCastExpression (CastExpression castExpression) + { + base.VisitCastExpression (castExpression); + + VisitTypeCastExpression (castExpression, ctx.Resolve (castExpression.Expression).Type, + ctx.ResolveType (castExpression.Type)); + } + + public override void VisitAsExpression (AsExpression asExpression) + { + base.VisitAsExpression (asExpression); + + VisitTypeCastExpression (asExpression, ctx.Resolve (asExpression.Expression).Type, + ctx.ResolveType (asExpression.Type)); + } + + void VisitTypeCastExpression (Expression expression, IType exprType, IType castToType) + { + if (TypeCompatibilityHelper.CheckTypeCompatibility (exprType, castToType) == + TypeCompatibilityHelper.TypeCompatiblity.NeverOfProvidedType) + AddIssue (expression, ctx.TranslateString ("Type cast expression of incompatible type")); + } + } + } +} diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ExpressionIsAlwaysOfProvidedTypeIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ExpressionIsAlwaysOfProvidedTypeIssue.cs index 8a75aad714..69b72d82ec 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ExpressionIsAlwaysOfProvidedTypeIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ExpressionIsAlwaysOfProvidedTypeIssue.cs @@ -56,7 +56,8 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring var type = ctx.Resolve (isExpression.Expression).Type; var providedType = ctx.ResolveType (isExpression.Type); - if (type.GetAllBaseTypes ().All (t => !t.Equals (providedType))) + if (TypeCompatibilityHelper.CheckTypeCompatibility (type, providedType) != + TypeCompatibilityHelper.TypeCompatiblity.AlwaysOfProvidedType) return; var action = new CodeAction (ctx.TranslateString ("Compare with 'null'"), diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ExpressionIsNeverOfProvidedTypeIssue.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ExpressionIsNeverOfProvidedTypeIssue.cs index c366c529e0..526b4ba7ee 100644 --- a/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ExpressionIsNeverOfProvidedTypeIssue.cs +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/CodeIssues/ExpressionIsNeverOfProvidedTypeIssue.cs @@ -49,22 +49,6 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring { } - void AddIssue(IsExpression isExpression) - { - AddIssue (isExpression, ctx.TranslateString ("Given expression is never of the provided type")); - } - - static bool CheckTypeParameterConstraints (IType type, IEnumerable baseTypes, - ITypeParameter typeParameter) - { - if (!typeParameter.DirectBaseTypes.All (t => baseTypes.Any (t2 => t2.Equals (t)))) - return false; - if (typeParameter.HasDefaultConstructorConstraint && - !type.GetConstructors (c => c.IsPublic && c.Parameters.Count == 0).Any ()) - return false; - return true; - } - public override void VisitIsExpression (IsExpression isExpression) { base.VisitIsExpression (isExpression); @@ -72,80 +56,9 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring var exprType = ctx.Resolve (isExpression.Expression).Type; var providedType = ctx.ResolveType (isExpression.Type); - var exprBaseTypes = exprType.GetAllBaseTypes ().ToArray (); - - // providedType is a base type of exprType - if (exprBaseTypes.Any (t => t.Equals (providedType))) - return; - - if ((exprType.IsReferenceType == true && providedType.IsReferenceType == false) || - (exprType.IsReferenceType == false && providedType.IsReferenceType == true)) { - AddIssue (isExpression); - return; - } - - var typeParameter = exprType as ITypeParameter; - var providedTypeParameter = providedType as ITypeParameter; - - if (typeParameter != null) { - // check if providedType can be a derived type - var providedBaseTypes = providedType.GetAllBaseTypes ().ToArray (); - var providedTypeDef = providedType.GetDefinition (); - // if providedType is sealed, check if it fullfills all the type parameter constraints, - // otherwise, only check if it is derived from EffectiveBaseClass - if (providedTypeParameter == null && (providedTypeDef == null || providedTypeDef.IsSealed)) { - if (CheckTypeParameterConstraints (providedType, providedBaseTypes, typeParameter)) - return; - } else if (providedBaseTypes.Any (t => t.Equals (typeParameter.EffectiveBaseClass))) { - return; - } - - // if providedType is also a type parameter, check if base classes are compatible - if (providedTypeParameter != null && - exprBaseTypes.Any (t => t.Equals (providedTypeParameter.EffectiveBaseClass))) - return; - - AddIssue (isExpression); - return; - } - // check if exprType fullfills all the type parameter constraints - if (providedTypeParameter != null && - CheckTypeParameterConstraints (exprType, exprBaseTypes, providedTypeParameter)) - return; - - switch (exprType.Kind) { - case TypeKind.Class: - var exprTypeDef = exprType.GetDefinition (); - if (exprTypeDef == null) - return; - // exprType is sealed, but providedType is not a base type of it or it does not - // fullfill all the type parameter constraints - if (exprTypeDef.IsSealed) - break; - - // check if providedType can be a derived type - if (providedType.Kind == TypeKind.Interface || - providedType.GetAllBaseTypes ().Any (t => t.Equals (exprType))) - return; - - if (providedTypeParameter != null && - exprBaseTypes.Any (t => t.Equals (providedTypeParameter.EffectiveBaseClass))) - return; - - break; - - case TypeKind.Struct: - case TypeKind.Delegate: - case TypeKind.Enum: - case TypeKind.Array: - case TypeKind.Anonymous: - case TypeKind.Null: - break; - - default: - return; - } - AddIssue (isExpression); + if (TypeCompatibilityHelper.CheckTypeCompatibility(exprType, providedType) == + TypeCompatibilityHelper.TypeCompatiblity.NeverOfProvidedType) + AddIssue (isExpression, ctx.TranslateString ("Given expression is never of the provided type")); } } } diff --git a/ICSharpCode.NRefactory.CSharp/Refactoring/TypeCompatibilityHelper.cs b/ICSharpCode.NRefactory.CSharp/Refactoring/TypeCompatibilityHelper.cs new file mode 100644 index 0000000000..98ae4e36fb --- /dev/null +++ b/ICSharpCode.NRefactory.CSharp/Refactoring/TypeCompatibilityHelper.cs @@ -0,0 +1,128 @@ +// +// TypeCompatibilityHelper.cs +// +// Author: +// Mansheng Yang +// +// Copyright (c) 2012 Mansheng Yang +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// 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.Collections.Generic; +using System.Linq; +using ICSharpCode.NRefactory.TypeSystem; + +namespace ICSharpCode.NRefactory.CSharp.Refactoring +{ + class TypeCompatibilityHelper + { + public enum TypeCompatiblity + { + MayOfProvidedType, + AlwaysOfProvidedType, + NeverOfProvidedType + } + + static bool CheckTypeParameterConstraints (IType type, IEnumerable baseTypes, + ITypeParameter typeParameter) + { + if (!typeParameter.DirectBaseTypes.All (t => baseTypes.Any (t2 => t2.Equals (t)))) + return false; + if (typeParameter.HasDefaultConstructorConstraint && + !type.GetConstructors (c => c.IsPublic && c.Parameters.Count == 0).Any ()) + return false; + return true; + } + + public static TypeCompatiblity CheckTypeCompatibility(IType exprType, IType providedType) + { + var exprBaseTypes = exprType.GetAllBaseTypes ().ToArray (); + + // providedType is a base type of exprType + if (exprBaseTypes.Any (t => t.Equals (providedType))) + return TypeCompatiblity.AlwaysOfProvidedType; + + if ((exprType.IsReferenceType == true && providedType.IsReferenceType == false) || + (exprType.IsReferenceType == false && providedType.IsReferenceType == true)) + return TypeCompatiblity.NeverOfProvidedType; + + var typeParameter = exprType as ITypeParameter; + var providedTypeParameter = providedType as ITypeParameter; + + if (typeParameter != null) { + // check if providedType can be a derived type + var providedBaseTypes = providedType.GetAllBaseTypes ().ToArray (); + var providedTypeDef = providedType.GetDefinition (); + // if providedType is sealed, check if it fullfills all the type parameter constraints, + // otherwise, only check if it is derived from EffectiveBaseClass + if (providedTypeParameter == null && (providedTypeDef == null || providedTypeDef.IsSealed)) { + if (CheckTypeParameterConstraints (providedType, providedBaseTypes, typeParameter)) + return TypeCompatiblity.MayOfProvidedType; + } else if (providedBaseTypes.Any (t => t.Equals (typeParameter.EffectiveBaseClass))) { + return TypeCompatiblity.MayOfProvidedType; + } + + // if providedType is also a type parameter, check if base classes are compatible + if (providedTypeParameter != null && + exprBaseTypes.Any (t => t.Equals (providedTypeParameter.EffectiveBaseClass))) + return TypeCompatiblity.MayOfProvidedType; + + return TypeCompatiblity.NeverOfProvidedType; + } + // check if exprType fullfills all the type parameter constraints + if (providedTypeParameter != null && + CheckTypeParameterConstraints (exprType, exprBaseTypes, providedTypeParameter)) + return TypeCompatiblity.MayOfProvidedType; + + switch (exprType.Kind) { + case TypeKind.Class: + var exprTypeDef = exprType.GetDefinition (); + if (exprTypeDef == null) + return TypeCompatiblity.MayOfProvidedType; + // exprType is sealed, but providedType is not a base type of it or it does not + // fullfill all the type parameter constraints + if (exprTypeDef.IsSealed) + break; + + // check if providedType can be a derived type + if (providedType.Kind == TypeKind.Interface || + providedType.GetAllBaseTypes ().Any (t => t.Equals (exprType))) + return TypeCompatiblity.MayOfProvidedType; + + if (providedTypeParameter != null && + exprBaseTypes.Any (t => t.Equals (providedTypeParameter.EffectiveBaseClass))) + return TypeCompatiblity.MayOfProvidedType; + + break; + + case TypeKind.Struct: + case TypeKind.Delegate: + case TypeKind.Enum: + case TypeKind.Array: + case TypeKind.Anonymous: + case TypeKind.Null: + break; + + default: + return TypeCompatiblity.MayOfProvidedType; + } + return TypeCompatiblity.NeverOfProvidedType; + } + } +} diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/CastExpressionOfIncompatibleTypeIssueTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/CastExpressionOfIncompatibleTypeIssueTests.cs new file mode 100644 index 0000000000..cdc813315d --- /dev/null +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/CastExpressionOfIncompatibleTypeIssueTests.cs @@ -0,0 +1,50 @@ +// +// CastExpressionOfIncompatibleTypeIssueTests.cs +// +// Author: +// Mansheng Yang +// +// Copyright (c) 2012 Mansheng Yang +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// 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 ICSharpCode.NRefactory.CSharp.Refactoring; +using NUnit.Framework; + +namespace ICSharpCode.NRefactory.CSharp.CodeIssues +{ + [TestFixture] + public class CastExpressionOfIncompatibleTypeIssueTests : InspectionActionTestBase + { + [Test] + public void Test () + { + var input = @" +class TestClass +{ + void TestMethod (int i) + { + var x = (string)i; + var y = i as string; + } +}"; + Test (input, 2); + } + } +} diff --git a/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj b/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj index 1d0390279d..529f0195ac 100644 --- a/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj +++ b/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj @@ -147,6 +147,7 @@ + @@ -392,14 +393,7 @@ False - - - - - - - - +