fix!: Allow empty strings during parsing to not trigger required. Instead do .Min(1).

[Oudwins]

Addresses #111

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of empty and nil values across validation and parsing, ensuring more accurate error reporting and default value application.
  • Adjusted environment variable retrieval to return nil for unset or empty values instead of an empty string.

• Tests

  • Updated test cases to reflect new validation rules and error code checks, particularly for required fields and minimum string lengths.
  • Added a new test verifying correct handling of optional and required struct fields during parsing.

[coderabbitai]

Walkthrough

The changes introduce stricter validation logic and input handling across multiple test suites and supporting code. Tests now consistently use nil instead of empty strings to represent missing or absent values. Validation assertions are updated to check error codes rather than error messages in several cases. The behavior of data provider Get methods is modified to return nil for missing or empty values rather than default zero values or empty strings. The function for detecting zero values is simplified to only treat nil as a zero value. A new test is added to verify correct handling of optional and required fields in struct parsing.

Changes

File(s)	Change Summary
i18n/i18n_test.go, string_custom_test.go, string_test.go	Updated test schemas to enforce stricter constraints (e.g., .Min(1)), changed test inputs from empty strings to nil, and updated assertions to check error codes or adapt to new validation logic.
numbers_custom_test.go, numbers_int64_test.go, numbers_test.go	Modified tests to check error codes instead of error messages for empty or whitespace string inputs; changed some test inputs from empty string to nil.
pointers_test.go	Changed test inputs from empty string to nil; test cases now include expected error code fields and assertions compare error codes instead of messages.
internals/DataProviders.go, zenv/zenv.go	Get methods now return nil for missing or empty values instead of zero values or empty strings, distinguishing between absent and present-but-empty data.
internals/zeroValues.go	Simplified zero value detection: only nil is considered a zero value, removing special handling for empty strings.
zenv/zenv_test.go	Updated test to expect nil instead of empty string when retrieving a non-existent environment variable.
struct_test.go	Added new test TestStructOptionalFields to verify optional and required field handling in struct parsing.

Sequence Diagram(s)

sequenceDiagram
    participant TestSuite
    participant Schema
    participant DataProvider
    participant Validator

    TestSuite->>Schema: Define schema (with required/min constraints)
    TestSuite->>Validator: Parse(input: nil or empty)
    Validator->>DataProvider: Get(key)
    DataProvider-->>Validator: Return nil if key missing/empty
    Validator->>Schema: Validate constraints (required/min)
    Schema-->>Validator: Return error with code if invalid
    Validator-->>TestSuite: Return error (checked by code)

Loading

Poem

In fields of code where values dwell,
A rabbit hops and sniffs the shell—
"Is it nil, or just a string?"
Now tests are sure what inputs bring.
With stricter checks and clearer calls,
The garden grows with fewer falls.
🐇✨

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6e02d32 and 754e4cf.

📒 Files selected for processing (12)

• i18n/i18n_test.go (4 hunks)
• internals/DataProviders.go (1 hunks)
• internals/zeroValues.go (1 hunks)
• numbers_custom_test.go (2 hunks)
• numbers_int64_test.go (1 hunks)
• numbers_test.go (1 hunks)
• pointers_test.go (2 hunks)
• string_custom_test.go (2 hunks)
• string_test.go (4 hunks)
• struct_test.go (1 hunks)
• zenv/zenv.go (1 hunks)
• zenv/zenv_test.go (1 hunks)

🧰 Additional context used

🧬 Code Graph Analysis (8)

numbers_custom_test.go (1)

zconst/consts.go (1)

• IssueCodeCoerce (71-71)

numbers_int64_test.go (1)

zconst/consts.go (1)

• IssueCodeCoerce (71-71)

i18n/i18n_test.go (3)

string.go (1)

• String (68-76)

internals/tests.go (1)

• Required (71-78)

utilsOptions.go (1)

• WithCtxValue (92-96)

numbers_test.go (1)

zconst/consts.go (1)

• IssueCodeCoerce (71-71)

string_test.go (2)

internals/tests.go (1)

• Required (71-78)

utilsOptions.go (1)

• Message (13-19)

string_custom_test.go (2)

internals/tests.go (1)

• Required (71-78)

zconst/consts.go (1)

• IssueCodeMin (89-89)

pointers_test.go (1)

zconst/consts.go (2)

• IssueCodeNotNil (67-67)
• ISSUE_KEY_ROOT (23-23)

struct_test.go (8)

internals/tests.go (2)

• In (126-143)
• Required (71-78)

time.go (1)

• Time (37-45)

struct.go (2)

• Struct (38-42)
• Schema (35-35)

string.go (1)

• String (68-76)

numbers.go (2)

• Int (72-80)
• Float (38-40)

boolean.go (1)

• Bool (34-42)

zconst/consts.go (1)

• IssueCodeRequired (63-63)

tutils/testIssueMessages.go (1)

• VerifyDefaultIssueMessagesMap (34-38)

🔇 Additional comments (29)

zenv/zenv_test.go (1)

85-85: Update test expectation to align with implementation change

This test has been correctly updated to expect nil instead of an empty string when retrieving a non-existent environment variable, reflecting the behavior change in zenv.go where the Get method now returns nil for empty or missing environment variables.

zenv/zenv.go (1)

20-26: Improved distinction between missing and empty values

This change modifies the Get method to return nil for empty environment variables instead of an empty string, which creates a clearer distinction between missing values and explicit empty strings during parsing. This aligns with the PR objective to not trigger the "required" validation for empty strings.

numbers_custom_test.go (2)

76-76: Updated test input to better represent missing value

Changed from using empty string to using nil as input, which better represents a missing value and aligns with the PR's objective to differentiate between empty strings and missing values.

---

86-86: Enhanced error checking precision

Changed from asserting the error message to asserting the error code, which provides a more precise validation of the error type. This helps ensure that the correct validation error is triggered for the input type.

internals/zeroValues.go (1)

17-17: Simplified zero value detection to support PR objective

The IsParseZeroValue function has been simplified to only treat nil as a zero value, removing the previous handling of empty strings. This is a key change that supports the PR objective of allowing empty strings during parsing without triggering the "required" validation.

This change aligns with the PR title "Allow empty strings during parsing to not trigger required. Instead do .Min(1)" by ensuring that only nil values (not empty strings) will be considered "zero" values during parsing, which means:

1. Required validation will only trigger for nil inputs
2. Empty strings will pass the required check but can be validated separately with .Min(1)

numbers_int64_test.go (2)

57-57: Improved error code validation

The test now correctly checks the error code rather than the error message when empty strings are parsed. This aligns with the PR objective of changing how empty strings are validated.

---

63-63: Improved error code validation for whitespace strings

Similarly for whitespace strings, the test now validates against error code (IssueCodeCoerce) rather than using error messages, which is more robust against message text changes.

numbers_test.go (2)

68-68: Improved error code validation

The test now correctly checks the error code rather than the error message when empty strings are parsed. This aligns with the PR objective of changing how empty strings are validated.

---

74-74: Improved error code validation for whitespace strings

Similarly for whitespace strings, the test now validates against error code (IssueCodeCoerce) rather than using error messages, which is more robust against message text changes.

string_custom_test.go (4)

10-10: Added necessary import for zconst

Added the import for the zconst package to support the new error code validation.

---

43-43: Core implementation of PR objective

This change directly implements the PR objective: empty strings now trigger a minimum length validation rather than failing on the "required" validation. The .Min(1) ensures that a string must have at least one character to be valid.

---

46-46: Updated assertion to match new validation behavior

The test now correctly asserts that empty strings fail with a minimum length error code (IssueCodeMin) rather than a required validation error, which is consistent with the PR objective.

---

50-50: Changed test input from empty string to nil

The test now uses nil instead of an empty string to test default value behavior, making a clear distinction between empty strings (which should be valid but fail .Min(1)) and missing values (nil).

string_test.go (5)

17-17: Changed test input from empty string to nil

The test now uses nil instead of an empty string to clearly distinguish between empty strings and absent values. This aligns with the PR objective where empty strings should be treated differently from missing values.

---

24-24: Changed test input from empty string to nil

This change is consistent with the overall approach in the PR, using nil to represent missing values in the tests rather than empty strings.

---

105-105: Changed test input from empty string to nil

Changed from empty string to nil to properly test the required validation with missing values rather than empty values.

---

119-119: Changed test input from empty string to nil

The test now correctly uses nil to test default value behavior with missing values rather than empty strings.

---

307-307: Fixed expected error message for empty strings

Empty strings now trigger a "one of" validation error rather than a "required" validation error, correctly reflecting that empty strings are now validated against the schema's validators rather than immediately failing the required check.

pointers_test.go (4)

15-15: Improved input validation by using nil instead of empty string.

This change aligns with the PR objective to distinguish between empty strings and missing values in validation. By using nil instead of an empty string, the test more clearly validates the "optional but not nil" behavior.

---

182-183: Enhanced test structure with explicit error code validation.

Adding the ErrCode field to the test case structure improves test clarity by explicitly stating the expected error code for each test case.

---

184-188: More precise test assertions with error code verification.

The updated test cases now precisely test the validation behavior by specifying expected error codes. This change shows that:

1. nil values trigger the IssueCodeNotNil error
2. Empty strings are now valid and don't trigger the "required" validation
3. Other non-nil values are also valid

This aligns perfectly with the PR objective.

---

193-193: Improved error validation with code-based assertions.

Changing from message-based assertions to code-based assertions makes the tests more robust against message wording changes and aligns with the goal of having consistent error identification throughout the codebase.

i18n/i18n_test.go (5)

33-33: Added string length validation to complement required check.

Adding .Min(1) to the schema validates the PR objective by ensuring empty strings don't trigger the "required" validation but are instead caught by a minimum length check.

---

47-47: Updated test inputs to use empty maps instead of maps with empty strings.

These changes correctly test the new validation behavior, where missing keys (rather than keys with empty strings) should trigger "required" validation errors.

Also applies to: 54-54, 61-61

---

79-82: Added safer input handling with existence check.

This improvement prevents potential panics when accessing missing keys in the input map, making the tests more robust.

---

88-88: Updated type assertion to use safely retrieved value.

This update correctly uses the safely retrieved value from lines 79-82, completing the safety improvement.

---

124-124: Changed input from empty string to nil for validation testing.

This change aligns with the PR objective by testing validation with a missing value (nil) rather than an empty string, which should now be considered valid.

internals/DataProviders.go (1)

74-80: Critical behavior change: returning nil for missing map keys.

This important change modifies how the MapDataProvider handles missing keys. Instead of returning the zero value of type T (which could be an empty string for string types), it now returns nil to indicate absence. This distinguishes between missing values and empty values, which is essential for the new validation behavior in the PR.

The implementation correctly:

1. Checks if the key exists in the map using the comma-ok idiom
2. Returns nil if the key doesn't exist
3. Returns the actual value if the key exists

This change is fundamental to the PR objective of treating empty strings differently from missing values.

struct_test.go (1)

132-165: Added comprehensive test for optional vs. required field validation.

This new test function thoroughly validates the core behavior targeted by the PR:

1. It creates a schema with both optional fields (without .Required()) and required fields
2. It tests that providing only the required fields results in no errors
3. It verifies that optional fields are properly set to zero values
4. It confirms that omitting required fields produces the correct error codes

The test is well-structured and provides good coverage of the functionality. It will help ensure the behavioral changes introduced by this PR don't regress in the future.

✨ Finishing Touches

• 📝 Generate Docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.