feat: add support for pkl files #164
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ package cli | |
| import ( | ||
| "fmt" | ||
| "os" | ||
| "os/exec" | ||
|
|
||
| "github.com/Boeing/config-file-validator/pkg/finder" | ||
| "github.com/Boeing/config-file-validator/pkg/reporter" | ||
|
|
@@ -73,6 +74,12 @@ func Init(opts ...Option) *CLI { | |
| return cli | ||
| } | ||
|
|
||
| // Check if the 'pkl' binary is present in the PATH | ||
| var isPklBinaryPresent = func() bool { | ||
| _, err := exec.LookPath("pkl") | ||
| return err == nil | ||
| } | ||
|
Comment on lines
+77
to
+81
There was a problem hiding this comment. I would move this as a method of pkl validator This would help you to validate it's behavior, abdband would prevent race in tests There was a problem hiding this comment. I briefly contemplated this but decided against it for 2 reasons:
There was a problem hiding this comment. I agree with the second point, it would need to rewrite a part of the code About the idea of perf, I don't mind here. If there is a pkl file, going to the pkl code to see if the file is valid, would require to validate its content. So it would be slow. Of course, when the pkl binary is not present, the code might have to go through the code and finally do not execute the code Please note you could consider simulating the binary presence, error, absence by calling arbitrary binary name:
It would be better than overloading the isPklBinaryPresent by something else I'm unsure it's a good idea, but why not, it may help There was a problem hiding this comment. I agree with @ccoVeille and feel like the validator should be able to validate if its enabled or not rather than having the check in the CLI. The validator will see the file, realize that it is not set up properly (no PKL) and then skip it and provide that feedback to the report. |
||
|
|
||
| // The Run method performs the following actions: | ||
| // - Finds the calls the Find method from the Finder interface to | ||
| // return a list of files | ||
|
|
@@ -88,6 +95,14 @@ func (c CLI) Run() (int, error) { | |
| } | ||
|
|
||
| for _, fileToValidate := range foundFiles { | ||
| // Pkl validation requires the 'pkl' binary to be present | ||
| if fileToValidate.FileType.Name == "pkl" { | ||
| if !isPklBinaryPresent() { | ||
| fmt.Printf("Warning: 'pkl' binary not found, file %s will be ignored.\n", fileToValidate.Path) | ||
| continue | ||
| } | ||
| } | ||
|
|
||
|
Comment on lines
+98
to
+105
There was a problem hiding this comment. I would move this in the pkl validator There was a problem hiding this comment. Let's go back to the reason I raised the point of doing something else that what is in this PR I see a problem in having a loop, then adding a dirty if hack in the loop. The validator should be in charge of validating, it's not the loop that should do a big |
||
| // read it | ||
| fileContent, err := os.ReadFile(fileToValidate.Path) | ||
| if err != nil { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| package validator | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
|
|
||
| "github.com/apple/pkl-go/pkl" | ||
| ) | ||
|
|
||
| // PklValidator is used to validate a byte slice that is intended to represent a | ||
| // PKL file. | ||
| type PklValidator struct{} | ||
|
|
||
| // Validate attempts to evaluate the provided byte slice as a PKL file. | ||
| func (PklValidator) Validate(b []byte) (bool, error) { | ||
| ctx := context.Background() | ||
|
|
||
| // Convert the byte slice to a ModuleSource using TextSource | ||
| source := pkl.TextSource(string(b)) | ||
|
There was a problem hiding this comment. Are there conditions where |
||
|
|
||
| evaluator, err := pkl.NewEvaluator(ctx, pkl.PreconfiguredOptions) | ||
| if err != nil { | ||
| return false, fmt.Errorf("failed to create evaluator: %w", err) | ||
| } | ||
|
|
||
| _, err = evaluator.EvaluateExpressionRaw(ctx, source, "") | ||
| if err != nil { | ||
| return false, fmt.Errorf("failed to evaluate module: %w", err) | ||
| } | ||
|
|
||
| return true, nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| name = "Swallow" | ||
|
There was a problem hiding this comment. Would prefer a more complete example |
||
|
|
||
| job { | ||
| title = "Sr. Nest Maker" | ||
| company = "Nests R Us" | ||
| yearsOfExperience = 2 | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| "invalid" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alphabetize please