Skip to content

Commit

Permalink
#9 Fix bug with not processed try-catch catch clause. Fix null refere…
Browse files Browse the repository at this point in the history 
…nce exception.

Update package version.
ExceptionAnalyzer minor refactoring.
  • Loading branch information
@denis-prodan
denis-prodan committed Dec 21, 2018
1 parent 70f63f0 commit 19cc02a
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 62 deletions.
2 changes: 1 addition & 1 deletion CleanCode.NET.VSIX/source.extension.vsixmanifest
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<PackageManifest Version="2.0.0" xmlns="http://schemas.microsoft.com/developer/vsx-schema/2011" xmlns:d="http://schemas.microsoft.com/developer/vsx-schema-design/2011">
<Metadata>
<Identity Id="CleanCode.NET.9ecfa9bb-0775-48d0-9898-4dbbbd529fe3" Version="0.4.4" Language="en-US" Publisher="Denys Prodan" />
<Identity Id="CleanCode.NET.9ecfa9bb-0775-48d0-9898-4dbbbd529fe3" Version="0.4.5" Language="en-US" Publisher="Denys Prodan" />
<DisplayName>Clean Code .NET</DisplayName>
<Description xml:space="preserve">Clean Code .NET - set of Roslyn analyzers aimed to improve code correctness
Not affiliated with Bob Martin</Description>
Expand Down
4 changes: 2 additions & 2 deletions CleanCodeNet.nuspec
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<package xmlns="http://schemas.microsoft.com/packaging/2012/06/nuspec.xsd">
<metadata>
<id>CleanCodeNet</id>
<version>0.3.4</version>
<version>0.3.5</version>
<authors>Denys Prodan</authors>
<owners>Denys Prodan</owners>
<requireLicenseAcceptance>false</requireLicenseAcceptance>
Expand All @@ -15,7 +15,7 @@
<dependency id="NamedParametersAnalyzer" version="0.3.1" />
<dependency id="SwitchAnalyzer" version="0.6.8" />
<dependency id="ConstructorNullAnalyzer" version="0.4.1" />
<dependency id="ExceptionsAnalyzer" version="0.1.4" />
<dependency id="ExceptionsAnalyzer" version="0.1.5" />
</dependencies>
</metadata>
<files>
Expand Down
51 changes: 51 additions & 0 deletions Exceptions/ExceptionsAnalyzer.Test/ExceptionsAnalyzerUnitTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,57 @@ public void NoVariableName_Correct()
VerifyCSharpDiagnostic(code);
}

[TestMethod]
public void InnerTryCatch_Correct()
{
var test = @"
catch (Exception e)
{
try
{
Console.WriteLine($""Exception happened! {e}"");
}
catch
{
throw;
}
}";
var code = BuildCode(test);

VerifyCSharpDiagnostic(code);
}

[TestMethod]
public void InnerTryCatch_Incorrect()
{
var test = @"
catch (Exception e)
{
try
{
var k = e;
}
catch(Exception inner)
{
throw new Exception();
}
}";
var code = BuildCode(test);

var expected = new DiagnosticResult
{
Id = Descriptors.RethrowWithoutInnerExceptionId,
Message = Descriptors.RethrowWithoutInnerMessage,
Severity = DiagnosticSeverity.Warning,
Locations =
new[]
{
new DiagnosticResultLocation("Test0.cs", 20, 17)
}
};

VerifyCSharpDiagnostic(code, expected);
}

private DiagnosticResult GetDiagnostic(string diagnosticId, string description, DiagnosticSeverity severity = DiagnosticSeverity.Warning)
{
Expand Down
154 changes: 98 additions & 56 deletions Exceptions/ExceptionsAnalyzer/ExceptionsAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,87 +108,129 @@ private static StatementAnalysisResult IsVariableUsedInStatement(string variable
{
if (statement is ExpressionStatementSyntax expression)
{
return IsVariableUsedInExpression(variableName, expression.Expression)
? StatementAnalysisResult.Used
: StatementAnalysisResult.NoUsage;
return IsVariableUsedInExpression(variableName, expression);
}

if (statement is LocalDeclarationStatementSyntax localDeclaration)
{
return localDeclaration.Declaration.Variables.Any(x => IsVariableUsedInExpression(variableName, x.Initializer.Value))
? StatementAnalysisResult.Used
: StatementAnalysisResult.NoUsage;
return IsVariableUsedInLocalDeclaration(variableName, localDeclaration);
}

if (statement is ThrowStatementSyntax throwStatement)
{
// "throw;" statement, good rethrow
if (throwStatement.Expression == null)
return StatementAnalysisResult.CorrectRethrow;
if (throwStatement.Expression != null)
{
if (throwStatement.Expression is IdentifierNameSyntax identifier)
{
if (identifier.Identifier.ValueText == variableName)
{
// "throw e" - leads to lost stack trace
return StatementAnalysisResult.RethrowSameException;
}
}

return IsVariableUsedInExpression(variableName, throwStatement.Expression)
? StatementAnalysisResult.CorrectRethrow // Rethrow with inner exception
: StatementAnalysisResult.RethrowWithoutInnerException;
}
return IsVariableUsedInThrow(variableName, throwStatement);
}

if (statement is ReturnStatementSyntax returnStatement)
{
return IsVariableUsedInExpression(variableName, returnStatement.Expression)
? StatementAnalysisResult.Used
: StatementAnalysisResult.NoUsage;
return IsVariableUsedInReturn(variableName, returnStatement);
}

if (statement is BlockSyntax blockStatement)
{
var statementsResults = blockStatement.Statements.Select(x => IsVariableUsedInStatement(variableName, x)).ToList();

var wrongUsageStatement = statementsResults.FirstOrDefault(x => x < StatementAnalysisResult.NoUsage);
if (wrongUsageStatement != StatementAnalysisResult.Undefined)
return wrongUsageStatement;

var correctUsageStatement = statementsResults.FirstOrDefault(x => x > StatementAnalysisResult.NoUsage);
if (correctUsageStatement != StatementAnalysisResult.Undefined)
return correctUsageStatement;

return StatementAnalysisResult.NoUsage;
return IsVariableUSedInBlock(variableName, blockStatement);
}

if (statement is IfStatementSyntax ifStatement)
{
// negative cases
var statementUsage = IsVariableUsedInStatement(variableName, ifStatement.Statement);
if (statementUsage < StatementAnalysisResult.NoUsage)
return statementUsage;
return IsVariableUsedInIf(variableName, ifStatement);
}

if (statement is TryStatementSyntax tryStatement)
{
return IsVariableUsedInStatement(variableName, tryStatement.Block);
}

var elseUsage = IsVariableUsedInStatement(variableName, ifStatement.Else.Statement);
if (elseUsage < StatementAnalysisResult.NoUsage)
return elseUsage;
throw new NotImplementedException($"Unknown statement type {statement}");
}

// positive cases
if (statementUsage > StatementAnalysisResult.NoUsage)
return statementUsage;
if (elseUsage > StatementAnalysisResult.NoUsage)
return elseUsage;
private static StatementAnalysisResult IsVariableUsedInThrow(string variableName,
ThrowStatementSyntax throwStatementSyntax)
{
// "throw;" statement, good rethrow
if (throwStatementSyntax.Expression == null)
return StatementAnalysisResult.CorrectRethrow;

// final decision if none above
var condiitionUsage = IsVariableUsedInExpression(variableName, ifStatement.Condition);
return condiitionUsage
? StatementAnalysisResult.Used
: StatementAnalysisResult.NoUsage;
if (throwStatementSyntax.Expression is IdentifierNameSyntax identifier)
{
if (identifier.Identifier.ValueText == variableName)
{
// "throw e" - leads to lost stack trace
return StatementAnalysisResult.RethrowSameException;
}
}

throw new NotImplementedException($"Unknown statement type {statement}");
return IsVariableUsedInExpression(variableName, throwStatementSyntax.Expression)
? StatementAnalysisResult.CorrectRethrow // Rethrow with inner exception
: StatementAnalysisResult.RethrowWithoutInnerException;
}

private static StatementAnalysisResult IsVariableUsedInIf(string variableName,
IfStatementSyntax ifStatementSyntax)
{
// negative cases
var statementUsage = IsVariableUsedInStatement(variableName, ifStatementSyntax.Statement);
if (statementUsage < StatementAnalysisResult.NoUsage)
return statementUsage;

var elseUsage = IsVariableUsedInStatement(variableName, ifStatementSyntax.Else.Statement);
if (elseUsage < StatementAnalysisResult.NoUsage)
return elseUsage;

// positive cases
if (statementUsage > StatementAnalysisResult.NoUsage)
return statementUsage;
if (elseUsage > StatementAnalysisResult.NoUsage)
return elseUsage;

// final decision if none above
var condiitionUsage = IsVariableUsedInExpression(variableName, ifStatementSyntax.Condition);
return condiitionUsage
? StatementAnalysisResult.Used
: StatementAnalysisResult.NoUsage;
}

private static StatementAnalysisResult IsVariableUSedInBlock(string variableName, BlockSyntax blockSyntax)
{
var statementsResults = blockSyntax.Statements.Select(x => IsVariableUsedInStatement(variableName, x)).ToList();

var wrongUsageStatement = statementsResults.FirstOrDefault(x => x < StatementAnalysisResult.NoUsage);
if (wrongUsageStatement != StatementAnalysisResult.Undefined)
return wrongUsageStatement;

var correctUsageStatement = statementsResults.FirstOrDefault(x => x > StatementAnalysisResult.NoUsage);
if (correctUsageStatement != StatementAnalysisResult.Undefined)
return correctUsageStatement;

return StatementAnalysisResult.NoUsage;
}

private static StatementAnalysisResult IsVariableUsedInReturn(string variableName,
ReturnStatementSyntax returnStatementSyntax)
{
return IsVariableUsedInExpression(variableName, returnStatementSyntax.Expression)
? StatementAnalysisResult.Used
: StatementAnalysisResult.NoUsage;
}

private static StatementAnalysisResult IsVariableUsedInLocalDeclaration(string variableName,
LocalDeclarationStatementSyntax localDeclarationStatementSyntax)
{
return localDeclarationStatementSyntax.Declaration.Variables.Any(x =>
IsVariableUsedInExpression(variableName, x.Initializer.Value))
? StatementAnalysisResult.Used
: StatementAnalysisResult.NoUsage;
}

private static StatementAnalysisResult IsVariableUsedInExpression(string variableName,
ExpressionStatementSyntax expressionStatementSyntax)
{
if (expressionStatementSyntax.Expression == null)
return StatementAnalysisResult.NoUsage;

return IsVariableUsedInExpression(variableName, expressionStatementSyntax.Expression)
? StatementAnalysisResult.Used
: StatementAnalysisResult.NoUsage;
}

private static bool IsVariableUsedInExpression(string variableName, ExpressionSyntax expression)
Expand Down
6 changes: 3 additions & 3 deletions Exceptions/ExceptionsAnalyzer/ExceptionsAnalyzer.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

<PropertyGroup>
<PackageId>ExceptionsAnalyzer</PackageId>
<PackageVersion>0.1.4.0</PackageVersion>
<PackageVersion>0.1.5.0</PackageVersion>
<Authors>Denys Prodan</Authors>
<PackageLicenseUrl></PackageLicenseUrl>
<PackageProjectUrl>https://github.com/denis-prodan/clean-code-net</PackageProjectUrl>
Expand All @@ -19,12 +19,12 @@
throw ex - leads to lost stack trace
Not used exception object
Rethrow new exception without inner exception</Description>
<PackageReleaseNotes>#9 Fix bug with not processed if statements in catch clause.</PackageReleaseNotes>
<PackageReleaseNotes>#9 Fix bug with not processed try-catch catch clause. Fix null reference exception.</PackageReleaseNotes>
<Copyright></Copyright>
<PackageTags>ExceptionsAnalyzer, analyzers</PackageTags>
<NoPackageAnalysis>true</NoPackageAnalysis>
<LangVersion>latest</LangVersion>
<Version>0.1.4</Version>
<Version>0.1.5</Version>
<Product>Exceptions Analyzer</Product>
</PropertyGroup>

Expand Down

0 comments on commit 19cc02a

Please sign in to comment.