Skip to content

Commit

Permalink
Merge pull request #285 from yoheimuta/fix-panic-due-to-mix-lineendin…
Browse files Browse the repository at this point in the history
…g/280

Fix indentrule causing runtime error when the file has a mix of LF an CRLF
yoheimuta authored Sep 23, 2022
2 parents 9ba4a39 + 5440322 commit 2e1d364
Showing 4 changed files with 95 additions and 40 deletions.
33 changes: 0 additions & 33 deletions .github/workflows/go.yml
Original file line number Diff line number Diff line change
@@ -26,36 +26,3 @@ jobs:

- name: Build
run: make build/cmd/protolint

- name: Build Docker image
run: docker build -t docker.io/yoheimuta/protolint:${{ github.sha }} .

- name: Run Trivy vulnerability scanner
uses: aquasecurity/[email protected]
with:
image-ref: docker.io/yoheimuta/protolint:${{ github.sha }}
exit-code: '1'
format: sarif
ignore-unfixed: true
output: trivy-results.sarif

- name: Upload Trivy scan results to GitHub Security tab
uses: github/codeql-action/upload-sarif@v2
if: always()
with:
sarif_file: trivy-results.sarif
wait-for-processing: true

- uses: hadolint/[email protected]
with:
dockerfile: Dockerfile
failure-threshold: style
format: sarif
output-file: hadolint-results.sarif

- name: Upload Hadolint scan results to GitHub Security tab
uses: github/codeql-action/upload-sarif@v2
if: always()
with:
sarif_file: hadolint-results.sarif
wait-for-processing: true
68 changes: 68 additions & 0 deletions _testdata/rules/indentrule/issue_280_mix_lineending.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// https://github.com/pseudomuto/protoc-gen-doc/blob/master/examples/proto/Booking.proto

/**
* Booking related messages.
*
* This file is really just an example. The data model is completely
* fictional.
*/
syntax = "proto3";

package com.example;

import "google/api/annotations.proto";
import "github.com/mwitkow/go-proto-validators/validator.proto";

/**
* Represents the booking status ID.
*/
message BookingStatusID {
int32 id = 1; /// Unique booking status ID.
}

/**
* Represents the status of a vehicle booking.
*/
message BookingStatus {
int32 id = 1; /// Unique booking status ID.
string description = 2 [(validator.field) = {string_not_empty: true, length_lt: 255}]; /// Booking status description. E.g. "Active".
}

/**
* Represents the booking of a vehicle.
*
* Vehicles are some cool shit. But drive carefully!
*/
message Booking {
int32 vehicle_id = 1; /// ID of booked vehicle.
int32 customer_id = 2; /// Customer that booked the vehicle.
BookingStatus status = 3; /// Status of the booking.

/** Has booking confirmation been sent? */
bool confirmation_sent = 4;

/** Has payment been received? */
bool payment_received = 5;

string color_preference = 6 [deprecated=true]; // Color preference of the customer.
}

// An empty message for testing
message EmptyBookingMessage {
}

/**
* Service for handling vehicle bookings.
*/
service BookingService {
/// Used to book a vehicle. Pass in a Booking and a BookingStatus will be returned.
rpc BookVehicle (Booking) returns (BookingStatus) {
option (google.api.http) = {
post: "/api/bookings/vehicle/{vehicle_id}"
body: "*"
};
}

/// Used to subscribe to updates of the BookingStatus.
rpc BookingUpdates (BookingStatusID) returns (stream BookingStatus);
}
19 changes: 19 additions & 0 deletions internal/addon/rules/indentRule_test.go
Original file line number Diff line number Diff line change
@@ -216,6 +216,25 @@ Fix https://github.com/yoheimuta/protolint/issues/139`,
),
},
},
{
name: `handle the case that the proto has a mixture of line ending formats like LF and CRLF.
Fix https://github.com/yoheimuta/protolint/issues/280`,
inputProtoPath: setting_test.TestDataPath("rules", "indentrule", "issue_280_mix_lineending.proto"),
wantFailures: []report.Failure{
report.Failuref(
meta.Position{
Filename: setting_test.TestDataPath("rules", "indentrule", "issue_280_mix_lineending.proto"),
Offset: 580,
Line: 27,
Column: 5,
},
"INDENT",
`Found an incorrect indentation style "%s". "%s" is correct.`,
" ",
" ",
),
},
},
}

for _, test := range tests {
15 changes: 8 additions & 7 deletions linter/fixer/fixer.go
Original file line number Diff line number Diff line change
@@ -60,13 +60,14 @@ func NewBaseFixing(protoFileName string) (*BaseFixing, error) {
if err != nil {
return nil, err
}
lineEnding, err := osutil.DetectLineEnding(string(content))
if err != nil {
return nil, err
}
if len(lineEnding) == 0 {
lineEnding = "\n"
}

// Regardless of the actual dominant line ending, the fixer will go with LF
// because the parser recognizes only LF as a line ending.
//
// It will work for most cases like used LF, CRLF, and a mix of LF and CRLF.
// See also https://github.com/yoheimuta/protolint/issues/280.
lineEnding := "\n"

return &BaseFixing{
content: content,
lineEnding: lineEnding,

0 comments on commit 2e1d364

Please sign in to comment.