fix(inputs): Validate that always-comment recive acceptable values
#80
Conversation
📝 WalkthroughWalkthroughThe changes add a new input parameter, Changes
Sequence Diagram(s)sequenceDiagram
participant Action as GitHub Action
participant Main as main.ts
participant Validator as Validation Logic
Action->>Main: Provide `always-comment` input
Main->>Validator: Validate input ("true"/"false")
alt Valid Input
Validator-->>Main: Return parsed boolean (alwaysCommentInput)
Main->>Main: Continue with execution
else Invalid Input
Validator-->>Main: Return error message
Main->>Main: Abort execution with error
end
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
Documentation and Community
|
always-comment recive acceptable valuesalways-comment recive acceptable values
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/main.ts (1)
214-220:⚠️ Potential issueFix the try-catch block syntax error.
The pipeline failures indicate there are issues with the try-catch block structure. According to the errors, there might be a missing closing brace or improper nesting. While the code excerpt appears correctly structured, the compiler is detecting syntax errors.
Ensure that all opening and closing braces are properly matched. Double-check if there are any invisible characters or formatting issues that might be affecting the syntax.
The specific errors point to line 214-220, suggesting issues with:
- Missing 'try' expected (line 214-214)
- Missing 'catch' or 'finally' expected (line 219-219)
- Missing '}' expected (line 220-220)
This could indicate that there might be an unclosed block or a syntax error in how the try-catch is constructed.
🧰 Tools
🪛 Biome (1.9.4)
[error] 214-214: Expected a statement but instead found 'catch (e)'.
Expected a statement here.
(parse)
[error] 218-219: Expected a catch clause but instead found 'run'.
Expected a catch clause here.
(parse)
[error] 219-219: expected
}but instead the file endsthe file ends here
(parse)
[error] 214-216: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
🪛 GitHub Check: check-dist
[failure] 214-214:
'try' expected.
[failure] 219-219:
'catch' or 'finally' expected.
[failure] 220-220:
'}' expected.🪛 GitHub Actions: Check `dist/`
[error] 214-214: TypeScript error TS1005: 'try' expected.
🪛 GitHub Actions: Tests
[error] 214-220: 'try' expected.
[error] 219-219: 'catch' or 'finally' expected.
[error] 220-220: '}' expected.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Check `dist/`
src/main.ts
[error] 214-214: TypeScript error TS1005: 'try' expected.
🪛 GitHub Actions: Tests
src/main.ts
[error] 214-220: 'try' expected.
[error] 219-219: 'catch' or 'finally' expected.
[error] 220-220: '}' expected.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: automerge
src/main.ts
Outdated
| const alwaysCommentInput = core.getInput('always-comment') | ||
| if (alwaysCommentInput.toLowerCase() !== 'true' || alwaysCommentInput.toLowerCase() !== 'false') { | ||
| error(`"always-comment" can contain "true" or "false", given "${alwaysCommentInput}"`) | ||
| // Convert always-comment input to boolean value. | ||
| // All values other than 'true' are considered false. | ||
| const alwaysComment = | ||
| core.getInput('always-comment').toLowerCase() === 'true' | ||
| const alwaysComment = alwaysCommentInput.toLowerCase() === 'true' |
There was a problem hiding this comment.
Fix logical error in the validation condition.
The current validation condition will always evaluate to true regardless of the input, causing the action to always fail. This is because the condition is checking if the input is not "true" OR not "false", which is always true for any value. This needs to be fixed using the && operator instead.
const alwaysCommentInput = core.getInput('always-comment')
-if (alwaysCommentInput.toLowerCase() !== 'true' || alwaysCommentInput.toLowerCase() !== 'false') {
+if (alwaysCommentInput.toLowerCase() !== 'true' && alwaysCommentInput.toLowerCase() !== 'false') {
error(`"always-comment" can contain "true" or "false", given "${alwaysCommentInput}"`)
}Alternatively, you could use a more explicit approach:
const alwaysCommentInput = core.getInput('always-comment')
+const normalizedInput = alwaysCommentInput.toLowerCase()
+if (normalizedInput !== 'true' && normalizedInput !== 'false') {
error(`"always-comment" can contain "true" or "false", given "${alwaysCommentInput}"`)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const alwaysCommentInput = core.getInput('always-comment') | |
| if (alwaysCommentInput.toLowerCase() !== 'true' || alwaysCommentInput.toLowerCase() !== 'false') { | |
| error(`"always-comment" can contain "true" or "false", given "${alwaysCommentInput}"`) | |
| // Convert always-comment input to boolean value. | |
| // All values other than 'true' are considered false. | |
| const alwaysComment = | |
| core.getInput('always-comment').toLowerCase() === 'true' | |
| const alwaysComment = alwaysCommentInput.toLowerCase() === 'true' | |
| const alwaysCommentInput = core.getInput('always-comment') | |
| if (alwaysCommentInput.toLowerCase() !== 'true' && alwaysCommentInput.toLowerCase() !== 'false') { | |
| error(`"always-comment" can contain "true" or "false", given "${alwaysCommentInput}"`) | |
| } | |
| // Convert always-comment input to boolean value. | |
| const alwaysComment = alwaysCommentInput.toLowerCase() === 'true' |
quickly done on phone, need to revisit, espetially
ifAlso, TODO: check are other inputs are validated as strixtly as possible.
And these validation & inputs code can be moved to separate function, and covered by unit tests
Summary by CodeRabbit
always-commentinput to prevent improper values from being processed.