diff --git a/ICSharpCode.NRefactory.CSharp.Refactoring/CodeActions/EnableExhaustivenessCheckAction.cs b/ICSharpCode.NRefactory.CSharp.Refactoring/CodeActions/EnableExhaustivenessCheckAction.cs new file mode 100644 index 000000000..4d97f142e --- /dev/null +++ b/ICSharpCode.NRefactory.CSharp.Refactoring/CodeActions/EnableExhaustivenessCheckAction.cs @@ -0,0 +1,33 @@ +namespace ICSharpCode.NRefactory.CSharp.Refactoring.CodeActions +{ + [ContextAction( + "Enable exhaustiveness check", + Description = "Enable exhaustiveness check for this switch")] + public class EnableExhaustivenessCheckAction: SpecializedCodeAction + { + protected override CodeAction GetAction(RefactoringContext context, SwitchStatement node) + { + if (!node.SwitchToken.Contains(context.Location)) + return null; + + if (SomeOfEnumValuesWasNotHandledInSwitchStatementIssue.IsExhaustivenessCheckEnabled(node)) + return null; + + var switchData = SomeOfEnumValuesWasNotHandledInSwitchStatementIssue.BuildSwitchData(node, context); + if (switchData == null) + return null; + + return new CodeAction( + context.TranslateString("Enabled exhaustiveness check"), + script => EnableCheck(script, node), + node + ); + } + + static void EnableCheck(Script script, SwitchStatement node) + { + var comment = new Comment(" " + SomeOfEnumValuesWasNotHandledInSwitchStatementIssue.EnableCheckComment); + script.InsertBefore(node, comment); + } + } +} diff --git a/ICSharpCode.NRefactory.CSharp.Refactoring/CodeIssues/Uncategorized/SomeOfEnumValuesWasNotHandledInSwitchStatementIssue.cs b/ICSharpCode.NRefactory.CSharp.Refactoring/CodeIssues/Uncategorized/SomeOfEnumValuesWasNotHandledInSwitchStatementIssue.cs new file mode 100644 index 000000000..dd6b23916 --- /dev/null +++ b/ICSharpCode.NRefactory.CSharp.Refactoring/CodeIssues/Uncategorized/SomeOfEnumValuesWasNotHandledInSwitchStatementIssue.cs @@ -0,0 +1,159 @@ +using System.Collections.Generic; +using System.Linq; +using ICSharpCode.NRefactory.Refactoring; +using ICSharpCode.NRefactory.Semantics; +using ICSharpCode.NRefactory.TypeSystem; + +namespace ICSharpCode.NRefactory.CSharp.Refactoring +{ + [IssueDescription("Some of enum values was not handled in switch statement", + Description = "Some of enum values was not handled in switch statement.", + Category = IssueCategories.CodeQualityIssues, + Severity = Severity.Warning)] + public class SomeOfEnumValuesWasNotHandledInSwitchStatementIssue: GatherVisitorCodeIssueProvider + { + internal static readonly string EnableCheckComment = "Check exhaustiveness"; + + internal static bool IsExhaustivenessCheckEnabled(SwitchStatement switchStatement) + { + var comment = switchStatement.Parent.GetChildByRole(Roles.Comment); + return comment != null && comment.Content.Trim() == EnableCheckComment; + } + + internal static SwitchData BuildSwitchData(SwitchStatement switchStatement, BaseRefactoringContext context) + { + return new SwitchDataBuilder(switchStatement, context).Build(); + } + + protected override IGatherVisitor CreateVisitor(BaseRefactoringContext context) + { + return new GatherVisitor(context); + } + + internal class SwitchData + { + public readonly IType EnumType; + public readonly IEnumerable LabelsExpressions; + + public SwitchData(IType enumType, IEnumerable labelsExpressions) + { + EnumType = enumType; + LabelsExpressions = labelsExpressions.ToArray(); + } + } + + class SwitchDataBuilder + { + readonly SwitchStatement _switchStatement; + readonly BaseRefactoringContext _context; + + public SwitchDataBuilder(SwitchStatement switchStatement, BaseRefactoringContext context) + { + _switchStatement = switchStatement; + _context = context; + } + + public SwitchData Build() + { + var enumType = GetEnumType(); + if (enumType == null) + return null; + + var labelsExpressions = GatherCaseLabelsExpressions(); + if (!AreAllExpressionsHasEnumType(labelsExpressions, enumType)) + return null; + + return new SwitchData(enumType, labelsExpressions.Cast()); + } + + IType GetEnumType() + { + var resolveResult = _context.Resolve(_switchStatement.Expression); + return resolveResult.Type.Kind == TypeKind.Enum ? resolveResult.Type : null; + } + + IEnumerable GatherCaseLabelsExpressions() + { + var labels = _switchStatement.SwitchSections.SelectMany(_ => _.CaseLabels); + var nonDefaultLabels = labels.Where(_ => !_.Expression.IsNull); + + return nonDefaultLabels.Select(_ => _.Expression); + } + + bool AreAllExpressionsHasEnumType(IEnumerable expressions, IType type) + { + var resolveResults = expressions.Select(_ => _context.Resolve(_)).ToArray(); + return resolveResults.Any() && resolveResults.All(_ => _.Type == type); + } + } + + class GatherVisitor: GatherVisitorBase + { + public GatherVisitor(BaseRefactoringContext ctx) : base(ctx) + { + } + + public override void VisitSwitchStatement(SwitchStatement switchStatement) + { + base.VisitSwitchStatement(switchStatement); + + if (IsExhaustivenessCheckEnabled(switchStatement)) { + var switchData = BuildSwitchData(switchStatement, ctx); + + if (switchData != null) { + var missingValues = GetMissingEnumValues(switchData).ToArray(); + + if (missingValues.Any()) + AddIssue(new CodeIssue( + switchStatement.SwitchToken, + "Some of enum values was not handled", + "Handle missing values", + script => GenerateMissingCasesForMissingValues(script, switchStatement, missingValues) + )); + } + } + } + + static IEnumerable GetMissingEnumValues(SwitchData switchData) + { + var handledValues = switchData.LabelsExpressions.Select(_ => _.MemberName).ToArray(); + var allValues = switchData.EnumType.GetFields(_ => _.IsConst && _.IsPublic); + + return allValues.Where(_ => !handledValues.Contains(_.Name)); + } + + static void GenerateMissingCasesForMissingValues(Script script, SwitchStatement switchStatement, IEnumerable values) + { + var astType = new SimpleType(values.First().Type.Name); + var newSwitchStatement = (SwitchStatement)switchStatement.Clone(); + + var previousSection = GetDefaultSection(newSwitchStatement); + foreach (var value in values.Reverse()) { + var newSection = new SwitchSection { + CaseLabels = { + new CaseLabel(new MemberReferenceExpression(astType.Clone(), value.Name)) + }, + Statements = { + new ThrowStatement(new ObjectCreateExpression(new SimpleType("System.NotImplementedException"))) + } + }; + + if (previousSection != null) + newSwitchStatement.SwitchSections.InsertBefore(previousSection, newSection); + else + newSwitchStatement.SwitchSections.Add(newSection); + + previousSection = newSection; + } + + script.Replace(switchStatement, newSwitchStatement); + } + + static SwitchSection GetDefaultSection(SwitchStatement switchStatement) + { + var sections = switchStatement.SwitchSections; + return sections.FirstOrDefault(s => s.CaseLabels.Any(l => l.Expression.IsNull)); + } + } + } +} diff --git a/ICSharpCode.NRefactory.CSharp.Refactoring/ICSharpCode.NRefactory.CSharp.Refactoring.csproj b/ICSharpCode.NRefactory.CSharp.Refactoring/ICSharpCode.NRefactory.CSharp.Refactoring.csproj index d52918256..7bb59eef5 100644 --- a/ICSharpCode.NRefactory.CSharp.Refactoring/ICSharpCode.NRefactory.CSharp.Refactoring.csproj +++ b/ICSharpCode.NRefactory.CSharp.Refactoring/ICSharpCode.NRefactory.CSharp.Refactoring.csproj @@ -424,6 +424,8 @@ + + diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/EnableExhaustivenessCheckActionTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/EnableExhaustivenessCheckActionTests.cs new file mode 100644 index 000000000..f6d203268 --- /dev/null +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeActions/EnableExhaustivenessCheckActionTests.cs @@ -0,0 +1,82 @@ +using NUnit.Framework; +using ICSharpCode.NRefactory.CSharp.Refactoring.CodeActions; + +namespace ICSharpCode.NRefactory.CSharp.CodeActions +{ + [TestFixture] + public class EnableExhaustivenessCheckActionTests: ContextActionTestBase + { + [Test] + public void TestNotEnumSwitchLabelsExpressions() + { + TestWrongContext(@" +enum Foo { First, Second, Third } + +class TestClass +{ + void TestMethod(Foo foo) + { + $switch (foo) + { + case 0: return; + case Foo.Second: return; + default: return; + } + } +}"); + } + + [Test] + public void TestCheckAlreadyEnabled() + { + TestWrongContext(@" +enum Foo { First, Second, Third } + +class TestClass +{ + void TestMethod(Foo foo) + { + // Check exhaustiveness + $switch (foo) + { + case Foo.Second: return; + default: return; + } + } +}"); + } + + [Test] + public void TestCheckDisabled() + { + Test(@" +enum Foo { First, Second, Third } + +class TestClass +{ + void TestMethod(Foo foo) + { + $switch (foo) + { + case Foo.Second: return; + default: return; + } + } +}", @" +enum Foo { First, Second, Third } + +class TestClass +{ + void TestMethod(Foo foo) + { + // Check exhaustiveness + switch (foo) + { + case Foo.Second: return; + default: return; + } + } +}"); + } + } +} diff --git a/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/SomeOfEnumValuesWasNotHandledInSwitchStatementIssueTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/SomeOfEnumValuesWasNotHandledInSwitchStatementIssueTests.cs new file mode 100644 index 000000000..eb39f734a --- /dev/null +++ b/ICSharpCode.NRefactory.Tests/CSharp/CodeIssues/SomeOfEnumValuesWasNotHandledInSwitchStatementIssueTests.cs @@ -0,0 +1,268 @@ +using System; +using NUnit.Framework; +using ICSharpCode.NRefactory.CSharp.Refactoring; + +namespace ICSharpCode.NRefactory.CSharp.CodeIssues +{ + [TestFixture] + public class SomeOfEnumValuesWasNotHandledInSwitchStatementIssueTests: InspectionActionTestBase + { + [Test] + public void TestNoComment() + { + TestWrongContext(@" +enum Foo { First, Second, Third } + +class TestClass +{ + void TestMethod(Foo foo) + { + $switch (foo) + { + case Foo.First: return; + default: return; + } + } +}"); + } + + [Test] + public void TestNotEnumSwitchExpression() + { + TestWrongContext(@" +enum Foo { First, Second, Third } + +class TestClass +{ + void TestMethod(object obj) + { + // Check exhaustiveness + $switch (obj) + { + case Foo.First: return; + default: return; + } + } +}"); + } + + [Test] + public void TestEmptySwitch() + { + TestWrongContext(@" +enum Foo { First, Second, Third } + +class TestClass +{ + void TestMethod(Foo foo) + { + // Check exhaustiveness + $switch (foo) + { + } + } +}"); + } + + [Test] + public void TestOnlyDefaultLabel() + { + TestWrongContext(@" +enum Foo { First, Second, Third } + +class TestClass +{ + void TestMethod(Foo foo) + { + // Check exhaustiveness + $switch (foo) + { + default: return; + } + } +}"); + } + + [Test] + public void TestNotEnumSwitchLabelsExpressions() + { + TestWrongContext(@" +enum Foo { First, Second, Third } + +class TestClass +{ + void TestMethod(Foo foo) + { + // Check exhaustiveness + $switch (foo) + { + case 0: return; + case Foo.Second: return; + default: return; + } + } +}"); + } + + [Test] + public void TestWrongEnumSwitchLabelsExpressions() + { + TestWrongContext(@" +enum Foo { First, Second, Third } +enum Bar { First, Second, Third } + +class TestClass +{ + void TestMethod(Foo foo) + { + // Check exhaustiveness + $switch (foo) + { + case Bar.First: return; + case Bar.Second: return; + default: return; + } + } +}"); + } + + [Test] + public void TestEnumExhaustive() + { + TestWrongContext(@" +enum Foo { First, Second, Third } + +class TestClass +{ + void TestMethod(Foo foo) + { + // Check exhaustiveness + $switch (foo) + { + case Foo.First: return; + case Foo.Second: return; + case Foo.Third: return; + default: return; + } + } +}"); + } + + [Test] + public void TestEnumNonExhaustiveWithoutDefaultCase() + { + Test( + "enum Foo { First, Second, Third }" + Environment.NewLine + + "" + Environment.NewLine + + "class TestClass" + Environment.NewLine + + "{" + Environment.NewLine + + "\tvoid TestMethod(Foo foo)" + Environment.NewLine + + "\t{" + Environment.NewLine + + "\t\t// Check exhaustiveness" + Environment.NewLine + + "\t\tswitch (foo)" + Environment.NewLine + + "\t\t{" + Environment.NewLine + + "\t\t\tcase Foo.Second: return;" + Environment.NewLine + + "\t\t}" + Environment.NewLine + + "\t}" + Environment.NewLine + + "}" + Environment.NewLine, + "enum Foo { First, Second, Third }" + Environment.NewLine + + "" + Environment.NewLine + + "class TestClass" + Environment.NewLine + + "{" + Environment.NewLine + + "\tvoid TestMethod(Foo foo)" + Environment.NewLine + + "\t{" + Environment.NewLine + + "\t\t// Check exhaustiveness" + Environment.NewLine + + "\t\tswitch (foo) {" + Environment.NewLine + + "\t\tcase Foo.Second:" + Environment.NewLine + + "\t\t\treturn;" + Environment.NewLine + + "\t\tcase Foo.First:" + Environment.NewLine + + "\t\t\tthrow new System.NotImplementedException ();" + Environment.NewLine + + "\t\tcase Foo.Third:" + Environment.NewLine + + "\t\t\tthrow new System.NotImplementedException ();" + Environment.NewLine + + "\t\t}" + Environment.NewLine + + "\t}" + Environment.NewLine + + "}" + Environment.NewLine + ); + } + + [Test] + public void TestEnumNonExhaustiveWithDefaultCase() + { + Test( + "enum Foo { First, Second, Third }" + Environment.NewLine + + "" + Environment.NewLine + + "class TestClass" + Environment.NewLine + + "{" + Environment.NewLine + + "\tvoid TestMethod(Foo foo)" + Environment.NewLine + + "\t{" + Environment.NewLine + + "\t\t// Check exhaustiveness" + Environment.NewLine + + "\t\tswitch (foo)" + Environment.NewLine + + "\t\t{" + Environment.NewLine + + "\t\t\tcase Foo.Second: return;" + Environment.NewLine + + "\t\t\tdefault: return;" + Environment.NewLine + + "\t\t}" + Environment.NewLine + + "\t}" + Environment.NewLine + + "}" + Environment.NewLine, + "enum Foo { First, Second, Third }" + Environment.NewLine + + "" + Environment.NewLine + + "class TestClass" + Environment.NewLine + + "{" + Environment.NewLine + + "\tvoid TestMethod(Foo foo)" + Environment.NewLine + + "\t{" + Environment.NewLine + + "\t\t// Check exhaustiveness" + Environment.NewLine + + "\t\tswitch (foo) {" + Environment.NewLine + + "\t\tcase Foo.Second:" + Environment.NewLine + + "\t\t\treturn;" + Environment.NewLine + + "\t\tcase Foo.First:" + Environment.NewLine + + "\t\t\tthrow new System.NotImplementedException ();" + Environment.NewLine + + "\t\tcase Foo.Third:" + Environment.NewLine + + "\t\t\tthrow new System.NotImplementedException ();" + Environment.NewLine + + "\t\tdefault:" + Environment.NewLine + + "\t\t\treturn;" + Environment.NewLine + + "\t\t}" + Environment.NewLine + + "\t}" + Environment.NewLine + + "}" + Environment.NewLine + ); + } + + [Test] + public void TestEnumNonExhaustiveWithDefaultCaseGroupedLabels() + { + Test( + "enum Foo { First, Second, Third }" + Environment.NewLine + + "" + Environment.NewLine + + "class TestClass" + Environment.NewLine + + "{" + Environment.NewLine + + "\tvoid TestMethod(Foo foo)" + Environment.NewLine + + "\t{" + Environment.NewLine + + "\t\t// Check exhaustiveness" + Environment.NewLine + + "\t\tswitch (foo)" + Environment.NewLine + + "\t\t{" + Environment.NewLine + + "\t\t\tcase Foo.Second:" + Environment.NewLine + + "\t\t\tcase Foo.First: return;" + Environment.NewLine + + "\t\t\tdefault: return;" + Environment.NewLine + + "\t\t}" + Environment.NewLine + + "\t}" + Environment.NewLine + + "}" + Environment.NewLine, + "enum Foo { First, Second, Third }" + Environment.NewLine + + "" + Environment.NewLine + + "class TestClass" + Environment.NewLine + + "{" + Environment.NewLine + + "\tvoid TestMethod(Foo foo)" + Environment.NewLine + + "\t{" + Environment.NewLine + + "\t\t// Check exhaustiveness" + Environment.NewLine + + "\t\tswitch (foo) {" + Environment.NewLine + + "\t\tcase Foo.Second:" + Environment.NewLine + + "\t\tcase Foo.First:" + Environment.NewLine + + "\t\t\treturn;" + Environment.NewLine + + "\t\tcase Foo.Third:" + Environment.NewLine + + "\t\t\tthrow new System.NotImplementedException ();" + Environment.NewLine + + "\t\tdefault:" + Environment.NewLine + + "\t\t\treturn;" + Environment.NewLine + + "\t\t}" + Environment.NewLine + + "\t}" + Environment.NewLine + + "}" + Environment.NewLine + ); + } + } +} diff --git a/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj b/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj index 40b7478c6..cbc3f5e10 100644 --- a/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj +++ b/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj @@ -612,6 +612,8 @@ + +