-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Chore/test empty string #64
Changes from all commits
d2dc0f9
37d8bd4
f156cbf
3fd4895
03d926b
2ff231c
1a0963a
cfee412
b03f298
c27d5e7
6fb7132
26e265b
2c8fc07
c372b6e
c8eaf08
629f351
6f16e90
f13fae7
9c5da5e
5813aef
19b73b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
import { Octokit } from "@octokit/rest"; | ||
import simpleGit from "simple-git"; | ||
|
||
const token = process.env.GITHUB_TOKEN; | ||
const [owner, repo] = process.env.GITHUB_REPOSITORY?.split("/") || []; | ||
const pullNumber = process.env.GITHUB_PR_NUMBER || process.env.PULL_REQUEST_NUMBER || "0"; | ||
const baseRef = process.env.GITHUB_BASE_REF; | ||
|
||
if (!token || !owner || !repo || pullNumber === "0" || !baseRef) { | ||
console.error("Missing required environment variables."); | ||
process.exit(1); | ||
} | ||
|
||
const octokit = new Octokit({ auth: token }); | ||
const git = simpleGit(); | ||
|
||
async function main() { | ||
try { | ||
// Get the base and head SHAs for the pull request | ||
const { data: pullRequest } = await octokit.pulls.get({ | ||
owner, | ||
repo, | ||
pull_number: parseInt(pullNumber), | ||
}); | ||
|
||
const baseSha = pullRequest.base.sha; | ||
const headSha = pullRequest.head.sha; | ||
|
||
// Fetch all remote branches and tags | ||
await git.fetch(["origin", baseSha, headSha]); | ||
|
||
// Get the diff of the pull request using the SHAs | ||
const diff = await git.diff([`${baseSha}...${headSha}`]); | ||
|
||
console.log("Checking for empty strings..."); | ||
const emptyStrings = parseDiffForEmptyStrings(diff); | ||
|
||
if (emptyStrings.length > 0) { | ||
console.error("Empty strings found:"); | ||
emptyStrings.forEach(({ file, line }) => { | ||
console.error(`${file}:${line}`); | ||
}); | ||
await createReview(emptyStrings); | ||
process.exit(1); // This line is causing the non-zero exit code | ||
} else { | ||
console.log("No empty strings found."); | ||
} | ||
} catch (error) { | ||
console.error("An error occurred:", error); | ||
process.exit(1); // This could also be causing the non-zero exit code | ||
} | ||
} | ||
|
||
function parseDiffForEmptyStrings(diff: string) { | ||
const violations: Array<{ file: string; line: number; content: string }> = []; | ||
const diffLines = diff.split("\n"); | ||
|
||
let currentFile = ""; | ||
let lineNumber = 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning Empty string found, consider another approach. Read more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning Empty string found, consider another approach. Read more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning Empty string found, consider another approach. Read more. |
||
|
||
diffLines.forEach((line) => { | ||
if (line.startsWith("+++ b/")) { | ||
currentFile = line.replace("+++ b/", ""); | ||
lineNumber = 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning Empty string found, consider another approach. Read more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning Empty string found, consider another approach. Read more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning Empty string found, consider another approach. Read more. |
||
} else if (line.startsWith("+") && !line.startsWith("+++")) { | ||
lineNumber++; | ||
if (line.includes('""')) { | ||
violations.push({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning Empty string found, consider another approach. Read more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning Empty string found, consider another approach. Read more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning Empty string found, consider another approach. Read more. |
||
file: currentFile, | ||
line: lineNumber, | ||
content: line.substring(1), | ||
}); | ||
} | ||
} else if (!line.startsWith("-")) { | ||
lineNumber++; | ||
} | ||
}); | ||
|
||
return violations; | ||
} | ||
|
||
async function createReview(violations: Array<{ file: string; line: number; content: string }>) { | ||
const annotationsBody = violations | ||
.map((v) => `${v.file}#L${v.line}\n\`\`\`suggestion\n${v.content.trim().replace('""', "/* TODO: Replace empty string */")}\n\`\`\``) | ||
.join("\n\n"); | ||
|
||
await octokit.pulls.createReview({ | ||
owner, | ||
repo, | ||
pull_number: parseInt(pullNumber), | ||
event: "COMMENT", | ||
body: `> [!WARNING]\n> ${violations.length} empty string${violations.length > 1 ? "s" : ""} detected in the code.\n\n${annotationsBody}\n\n[Read more about empty string issues](https://www.github.com/ubiquity/ts-template/issues/31).`, | ||
}); | ||
} | ||
|
||
main().catch((error) => { | ||
console.error("Error running empty string check:", error); | ||
process.exit(1); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,71 +1,32 @@ | ||
name: Check for Empty Strings | ||
name: Empty String Check | ||
|
||
on: | ||
pull_request: | ||
types: [opened, synchronize, reopened] | ||
|
||
jobs: | ||
check-empty-strings: | ||
check-for-empty-strings: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- name: Checkout code | ||
uses: actions/checkout@v4 | ||
|
||
- uses: actions/checkout@v4 | ||
- name: Setup Node.js | ||
uses: actions/setup-node@v4 | ||
with: | ||
node-version: "20.10.0" | ||
- name: Get GitHub App token | ||
uses: tibdex/[email protected] | ||
id: get_installation_token | ||
with: | ||
app_id: ${{ secrets.APP_ID }} | ||
private_key: ${{ secrets.APP_PRIVATE_KEY }} | ||
|
||
- name: Find empty strings | ||
id: find-empty-strings | ||
- name: Install Dependencies | ||
run: | | ||
# Get the base branch (usually main or master) | ||
BASE_BRANCH=$(gh pr view ${{ github.event.pull_request.number }} --json baseRefName --jq .baseRefName) | ||
|
||
# Get the list of changed files in the PR | ||
CHANGED_FILES=$(gh pr diff ${{ github.event.pull_request.number }} --name-only) | ||
|
||
# Initialize empty string for results | ||
EMPTY_STRINGS="" | ||
|
||
# Loop through changed files | ||
for file in $CHANGED_FILES; do | ||
if [[ $file =~ \.(ts|tsx)$ ]]; then | ||
# Get the diff for this file | ||
DIFF=$(gh pr diff ${{ github.event.pull_request.number }} --color never -- $file) | ||
|
||
# Find added lines with empty strings | ||
ADDED_EMPTY=$(echo "$DIFF" | grep -n "^+" | grep -E "(''{2}|\"\"{2})" | sed "s/^+//g" | sed "s/^/${file}:/") | ||
|
||
# Append results | ||
if [ ! -z "$ADDED_EMPTY" ]; then | ||
EMPTY_STRINGS+="$ADDED_EMPTY"$'\n' | ||
fi | ||
fi | ||
done | ||
|
||
# Output results | ||
echo "EMPTY_STRINGS<<EOF" >> $GITHUB_OUTPUT | ||
echo "$EMPTY_STRINGS" >> $GITHUB_OUTPUT | ||
echo "EOF" >> $GITHUB_OUTPUT | ||
yarn add tsx simple-git | ||
- name: Check for Empty Strings | ||
run: | | ||
yarn tsx .github/empty-string-checker.ts | ||
env: | ||
GH_TOKEN: ${{ steps.get_installation_token.outputs.token }} | ||
|
||
- name: Comment on PR | ||
uses: actions/github-script@v6 | ||
if: steps.find-empty-strings.outputs.EMPTY_STRINGS | ||
with: | ||
github-token: ${{ steps.get_installation_token.outputs.token }} | ||
script: | | ||
const emptyStrings = process.env.EMPTY_STRINGS.trim().split('\n'); | ||
if (emptyStrings.length > 0) { | ||
const body = `### Empty Strings Found\n\nPlease review the following lines containing empty strings:\n\n${emptyStrings.map(line => `- \`${line}\``).join('\n')}`; | ||
github.rest.issues.createComment({ | ||
issue_number: context.issue.number, | ||
owner: context.repo.owner, | ||
repo: context.repo.name, | ||
body: body | ||
}); | ||
} | ||
GITHUB_TOKEN: ${{ steps.get_installation_token.outputs.token }} | ||
GITHUB_REPOSITORY: ${{ github.repository }} | ||
GITHUB_PR_NUMBER: ${{ github.event.pull_request.number }} | ||
GITHUB_BASE_REF: ${{ github.base_ref }} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,3 +2,6 @@ import * as dotenv from "dotenv"; | |
// load environment variables (if you have them) | ||
dotenv.config(); | ||
console.log("Welcome to ts-template"); | ||
|
||
const _EMPTY_STRING_TEST_1 = ""; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning Empty string found, consider another approach. Read more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning Empty string found, consider another approach. Read more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning Empty string found, consider another approach. Read more. |
||
const _EMPTY_STRING_TEST_2 = ""; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning: Empty string found.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning Empty string found, consider another approach. Read more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning Empty string found, consider another approach. Read more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning Empty string found, consider another approach. Read more. |
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.
Warning: Empty string found.