Skip to content
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

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 99 additions & 0 deletions .github/empty-string-checker.ts
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;
Copy link
Contributor

Choose a reason for hiding this comment

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

Warning: Empty string found.

  let currentFile = "";

Copy link
Contributor

Choose a reason for hiding this comment

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

Warning: Empty string found.

  let currentFile = "";

Copy link
Member Author

Choose a reason for hiding this comment

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

Warning: Empty string found.

  let currentFile = "";

Copy link
Member Author

Choose a reason for hiding this comment

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

Warning: Empty string found.

  let currentFile = "";

Copy link
Member Author

Choose a reason for hiding this comment

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

Warning: Empty string found.

  let currentFile = "";

Copy link
Contributor

Choose a reason for hiding this comment

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

Warning: Empty string found.

  let currentFile = "";

Copy link
Contributor

Choose a reason for hiding this comment

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

Warning

Empty string found, consider another approach. Read more.

Choose a reason for hiding this comment

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

Warning

Empty string found, consider another approach. Read more.

Choose a reason for hiding this comment

The 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;
Copy link
Contributor

Choose a reason for hiding this comment

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

Warning: Empty string found.

      currentFile = line.replace("+++ b/", "");

Copy link
Contributor

Choose a reason for hiding this comment

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

Warning: Empty string found.

      currentFile = line.replace("+++ b/", "");

Copy link
Member Author

Choose a reason for hiding this comment

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

Warning: Empty string found.

      currentFile = line.replace("+++ b/", "");

Copy link
Member Author

Choose a reason for hiding this comment

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

Warning: Empty string found.

      currentFile = line.replace("+++ b/", "");

Copy link
Member Author

Choose a reason for hiding this comment

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

Warning: Empty string found.

      currentFile = line.replace("+++ b/", "");

Copy link
Contributor

Choose a reason for hiding this comment

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

Warning: Empty string found.

      currentFile = line.replace("+++ b/", "");

Copy link
Contributor

Choose a reason for hiding this comment

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

Warning

Empty string found, consider another approach. Read more.

Choose a reason for hiding this comment

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

Warning

Empty string found, consider another approach. Read more.

Choose a reason for hiding this comment

The 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({
Copy link
Contributor

Choose a reason for hiding this comment

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

Warning: Empty string found.

      if (line.includes('""')) {

Copy link
Contributor

Choose a reason for hiding this comment

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

Warning: Empty string found.

      if (line.includes('""')) {

Copy link
Member Author

Choose a reason for hiding this comment

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

Warning: Empty string found.

      if (line.includes('""')) {

Copy link
Member Author

Choose a reason for hiding this comment

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

Warning: Empty string found.

      if (line.includes('""')) {

Copy link
Member Author

Choose a reason for hiding this comment

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

Warning: Empty string found.

      if (line.includes('""')) {

Copy link
Contributor

Choose a reason for hiding this comment

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

Warning: Empty string found.

      if (line.includes('""')) {

Copy link
Contributor

Choose a reason for hiding this comment

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

Warning

Empty string found, consider another approach. Read more.

Choose a reason for hiding this comment

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

Warning

Empty string found, consider another approach. Read more.

Choose a reason for hiding this comment

The 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);
});
2 changes: 1 addition & 1 deletion .github/knip.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { KnipConfig } from "knip";

const config: KnipConfig = {
entry: ["build/index.ts"],
entry: ["build/index.ts", ".github/empty-string-checker.ts"],
project: ["src/**/*.ts"],
ignore: ["src/types/config.ts", "**/__mocks__/**", "**/__fixtures__/**"],
ignoreExportsUsedInFile: true,
Expand Down
71 changes: 16 additions & 55 deletions .github/workflows/no-empty-strings.yml
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 }}
4 changes: 1 addition & 3 deletions .github/workflows/sync-template.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ jobs:
with:
app_id: ${{ secrets.APP_ID }}
private_key: ${{ secrets.APP_PRIVATE_KEY }}

- name: Sync branch to template
env:
GH_TOKEN: ${{ steps.get_installation_token.outputs.token }}
Expand All @@ -45,5 +45,3 @@ jobs:
git commit -m "chore: sync template"
git push "$original_remote" "$pr_branch"
gh pr create --title "Sync branch to template" --body "This pull request merges changes from the template repository." --head "$pr_branch" --base "$branch_name"


3 changes: 3 additions & 0 deletions build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "";
Copy link
Contributor

Choose a reason for hiding this comment

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

Warning: Empty string found.

const _EMPTY_STRING_TEST_1 = "";

Copy link
Contributor

Choose a reason for hiding this comment

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

Warning: Empty string found.

const _EMPTY_STRING_TEST_1 = "";

Copy link
Member Author

Choose a reason for hiding this comment

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

Warning: Empty string found.

const _EMPTY_STRING_TEST_1 = "";

Copy link
Member Author

Choose a reason for hiding this comment

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

Warning: Empty string found.

const _EMPTY_STRING_TEST_1 = "";

Copy link
Member Author

Choose a reason for hiding this comment

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

Warning: Empty string found.

const _EMPTY_STRING_TEST_1 = "";

Copy link
Contributor

Choose a reason for hiding this comment

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

Warning: Empty string found.

const _EMPTY_STRING_TEST_1 = "";

Copy link
Contributor

Choose a reason for hiding this comment

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

Warning

Empty string found, consider another approach. Read more.

Choose a reason for hiding this comment

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

Warning

Empty string found, consider another approach. Read more.

Choose a reason for hiding this comment

The 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 = "";
Copy link
Contributor

Choose a reason for hiding this comment

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

Warning: Empty string found.

const _EMPTY_STRING_TEST_2 = "";

Copy link
Contributor

Choose a reason for hiding this comment

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

Warning: Empty string found.

const _EMPTY_STRING_TEST_2 = "";

Copy link
Member Author

Choose a reason for hiding this comment

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

Warning: Empty string found.

const _EMPTY_STRING_TEST_2 = "";

Copy link
Member Author

Choose a reason for hiding this comment

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

Warning: Empty string found.

const _EMPTY_STRING_TEST_2 = "";

Copy link
Member Author

Choose a reason for hiding this comment

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

Warning: Empty string found.

const _EMPTY_STRING_TEST_2 = "";

Copy link
Contributor

Choose a reason for hiding this comment

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

Warning: Empty string found.

const _EMPTY_STRING_TEST_2 = "";

Copy link
Contributor

Choose a reason for hiding this comment

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

Warning

Empty string found, consider another approach. Read more.

Choose a reason for hiding this comment

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

Warning

Empty string found, consider another approach. Read more.

Choose a reason for hiding this comment

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

Warning

Empty string found, consider another approach. Read more.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
"@cspell/dict-typescript": "^3.1.2",
"@jest/globals": "29.7.0",
"@mswjs/data": "0.16.1",
"@octokit/rest": "^21.0.2",
"@types/jest": "29.5.12",
"@types/node": "^20.11.19",
"@typescript-eslint/eslint-plugin": "^7.0.1",
Expand All @@ -60,6 +61,7 @@
"lint-staged": "^15.2.2",
"npm-run-all": "^4.1.5",
"prettier": "^3.2.5",
"simple-git": "^3.27.0",
"ts-jest": "29.1.2",
"tsx": "^4.7.1",
"typescript": "^5.3.3"
Expand Down
Loading
Loading