Skip to content

Add simple cli to validate expressions#137

Open
ahpook wants to merge 3 commits intomainfrom
ahpook/cli
Open

Add simple cli to validate expressions#137
ahpook wants to merge 3 commits intomainfrom
ahpook/cli

Conversation

@ahpook
Copy link

@ahpook ahpook commented Mar 6, 2026

This adds a little CLI tool called spdx-validate, which uses the ValidateLicenses function from the library to process spdx expressions from a file or STDIN, checking each line for validity.

(This PR is re-creating #136 from a local branch instead of a fork)

ahpook added 3 commits March 4, 2026 10:55
This adds a simple cli to the package called `spdx-validate`.  It takes a string or an input file containing spdx expressions and runs the `ValidateLicenses` function on them, presenting errors if they fail validation.

(Disclosure: Coded with Claude Opus 4.6)
The previous split code path was left over from my initial attempts. now stdin is treated like a newline-separated file rather than being special-cased, so the tool can accept multiple lines from a shell pipeline or a file.
@ahpook ahpook requested review from dangoor and elrayle as code owners March 6, 2026 01:21
Copilot AI review requested due to automatic review settings March 6, 2026 01:21
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a small spdx-validate CLI (under cmd/spdx-validate) to validate newline-separated SPDX license expressions via the existing spdxexp.ValidateLicenses library function, with accompanying documentation and tests.

Changes:

  • Introduce spdx-validate Cobra-based CLI that reads from stdin or a --file input and reports invalid expressions.
  • Add unit/integration-style tests for the CLI’s core validation routine.
  • Document build/run usage in the README and add required Go module dependencies.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
cmd/spdx-validate/main.go Implements the CLI command and validateExpressions processing loop.
cmd/spdx-validate/main_test.go Adds tests covering valid/invalid inputs and reading from a temp file.
README.md Documents how to build and use spdx-validate.
go.mod Adds Cobra dependency (and related indirect deps).
go.sum Records checksums for newly added dependencies.
Comments suppressed due to low confidence (2)

cmd/spdx-validate/main.go:60

  • The doc comment says (false, err) is returned “on read errors or when no expressions are found”, but the implementation also returns an error when all (non-blank) expressions are invalid. Update the comment so it matches the actual behavior (or adjust the behavior to match the comment).
// validateExpressions reads newline-separated SPDX expressions from r,
// validates each one, and writes error messages to w for any that are invalid.
// Returns (true, nil) when all are valid, (false, nil) when any are invalid, or
// (false, err) on read errors or when no expressions are found.
func validateExpressions(r io.Reader, w io.Writer) (bool, error) {

cmd/spdx-validate/main.go:70

  • lineNum is incremented before trimming/skipping blank lines, but later you treat lineNum as the number of expressions processed (e.g., in the all-invalid/empty logic and the summary). This makes inputs with blank lines behave incorrectly (e.g., a file with only blank lines can return success, and summaries will count blank lines as “expressions”). Track a separate exprCount for non-blank lines, use lineNum only for reporting original line numbers, and base failures==... and the summary denominator on exprCount.
		lineNum++
		line := strings.TrimSpace(scanner.Text())
		if line == "" {
			continue
		}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants