Skip to content

Commit

Permalink
(GH-557) Fix exception for issues across multiple lines (#558)
Browse files Browse the repository at this point in the history
  • Loading branch information
pascalberger authored Apr 25, 2024
1 parent 4d13317 commit aef175f
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@

<ItemGroup>
<None Remove="Testfiles\error.json" />
<None Remove="Testfiles\warning-across-multiple-lines.json" />
</ItemGroup>

<ItemGroup>
<EmbeddedResource Include="Testfiles\warning-across-multiple-lines.json" />
<EmbeddedResource Include="Testfiles\error.json" />
<EmbeddedResource Include="Testfiles\basic.json" />
</ItemGroup>
Expand Down
25 changes: 25 additions & 0 deletions src/Cake.Issues.Terraform.Tests/TerraformProviderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,31 @@ public void Should_Read_Error_Correct()
.OfRule("Could not load plugin")
.WithPriority(IssuePriority.Error));
}

[SkippableFact]
public void Should_Read_Warning_Across_Multiple_Lines_Correct()
{
// Uses Windows specific paths.
Skip.IfNot(RuntimeInformation.IsOSPlatform(OSPlatform.Windows));

// Given
var fixture = new TerraformProviderFixture("warning-across-multiple-lines.json", @"./");

// When
var issues = fixture.ReadIssues().ToList();

// Then
issues.Count.ShouldBe(1);
IssueChecker.Check(
issues[0],
IssueBuilder.NewIssue(
"This feature is deprecated and will be removed in a major release. Please use the `lifecycle.ignore_changes` argument to specify the fields in `body` to ignore.",
"Cake.Issues.Terraform.TerraformIssuesProvider",
"Terraform")
.InFile("modules\\deployment-ring\\storage-account.tf", 84, 86, 25, 4)
.OfRule("Attribute Deprecated")
.WithPriority(IssuePriority.Warning));
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"format_version": "1.0",
"valid": true,
"error_count": 0,
"warning_count": 1,
"diagnostics": [
{
"severity": "warning",
"summary": "Attribute Deprecated",
"detail": "This feature is deprecated and will be removed in a major release. Please use the `lifecycle.ignore_changes` argument to specify the fields in `body` to ignore.",
"address": "module.deployment-ring.azapi_update_resource.storage_account_data_blob_settings",
"range": {
"filename": "modules\\deployment-ring\\storage-account.tf",
"start": {
"line": 84,
"column": 25,
"byte": 2092
},
"end": {
"line": 86,
"column": 4,
"byte": 2121
}
},
"snippet": {
"context": "resource \"azapi_update_resource\" \"storage_account_data_blob_settings\"",
"code": " ignore_body_changes = [\n \"properties.cors\"\n ]",
"start_line": 84,
"highlight_start_offset": 24,
"highlight_end_offset": 51,
"values": []
}
}
]
}
57 changes: 56 additions & 1 deletion src/Cake.Issues.Tests/IssueTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2374,7 +2374,7 @@ public void Should_Throw_If_EndColumn_Is_Smaller_Column()
const string projectName = "foo";
const string filePath = @"src\foo.cs";
const int line = 10;
const int endLine = 12;
const int endLine = 10;
const int column = 50;
const int endColumn = 5;
var fileLink = new Uri("https://github.com/myorg/myrepo/blob/develop/src/foo.cs#L10-L12");
Expand Down Expand Up @@ -2530,6 +2530,61 @@ public void Should_Handle_EndColumn_Which_Is_Equals_Column()
issue.EndColumn.ShouldBe(endColumn);
}

[Fact]
public void Should_Handle_EndColumn_Which_Is_Smaller_Than_Column_If_EndLine_Is_Higher()
{
// Given
const string identifier = "identifier";
const string projectPath = @"src\foo.csproj";
const string projectName = "foo";
const string filePath = @"src\foo.cs";
const int line = 10;
const int endLine = 12;
const int column = 50;
const int endColumn = 10;
var fileLink = new Uri("https://github.com/myorg/myrepo/blob/develop/src/foo.cs#L10-L12");
const string messageText = "MessageText";
const string messageHtml = "MessageHtml";
const string messageMarkdown = "MessageMarkdown";
const int priority = 1;
const string priorityName = "Warning";
const string rule = "Rule";
const string ruleName = "Rule Name";
var ruleUri = new Uri("https://google.com");
const string providerType = "ProviderType";
const string providerName = "ProviderName";
const string run = "Run";
var additionalInformation = new Dictionary<string, string>();

// When
var issue =
new Issue(
identifier,
projectPath,
projectName,
filePath,
line,
endLine,
column,
endColumn,
fileLink,
messageText,
messageHtml,
messageMarkdown,
priority,
priorityName,
rule,
ruleName,
ruleUri,
run,
providerType,
providerName,
additionalInformation);

// Then
issue.EndColumn.ShouldBe(endColumn);
}

[Theory]
[InlineData(null)]
[InlineData(1)]
Expand Down
3 changes: 2 additions & 1 deletion src/Cake.Issues/Issue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ public Issue(
throw new ArgumentOutOfRangeException(nameof(endColumn), "Cannot specify the end of column range while not specifying start of column range.");
}

if (column.HasValue && endColumn.HasValue && column.Value > endColumn.Value)
// End column is not allowed to be before start column, except if issue is across multiple lines.
if (column.HasValue && endColumn.HasValue && (!endLine.HasValue || endLine.Value == line.Value) && column.Value > endColumn.Value)
{
throw new ArgumentOutOfRangeException(nameof(endColumn), "Column range needs to end after start of range.");
}
Expand Down

0 comments on commit aef175f

Please sign in to comment.