Skip to content

Commit

Permalink
Lint changed files by default in pre-push hook
Browse files Browse the repository at this point in the history
Currently if you install the pre-push hook by running `yarn
setup-git-hooks`, then each time you push, every file that can be linted
will be. This is incredibly time-consuming if you want to push a branch
and you've only changed a handful of files on your branch.

To address this, this commit adds a shell script which replaces the
existing `lint` and `lint:fix` package scripts. This new script not only
runs in "check" and "fix" modes (that is, either check and exit without
fixes, or check and automatically make fixes), it also has the ability
to detect which files have changed within the branch that you're on and
only run ESLint and Prettier on those files.

To accomplish this, the argument to `eslint` and `prettier` have been
simplified so that instead of excluding non-lintable files when these
commands are called, ignore files are used instead. This required
updating to Prettier 3, which reads from both `.prettierignore` and
`.gitignore`, a useful task Prettier 2 did not do.

Of course, you don't need to install the Git hook to take advantage of
the changes in this commit. The `lint` and `lint:fix` package scripts
now take advantage of the new shell script, and there are also two new
package scripts that delegate to the script, namely, `lint:only-branch`
and `lint:fix:only-branch`.

Finally, CI will still run `yarn lint` to ensure that all files in the
repo still pass lint.
  • Loading branch information
mcmire committed May 7, 2024
1 parent 02de743 commit f6072a8
Show file tree
Hide file tree
Showing 7 changed files with 360 additions and 208 deletions.
4 changes: 4 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ module.exports = {
extends: ['@metamask/eslint-config', '@metamask/eslint-config-nodejs'],
ignorePatterns: [
'!.eslintrc.js',
'!.prettierrc.js',
'!jest.config.js',
'node_modules',
'**/dist',
Expand Down Expand Up @@ -127,5 +128,8 @@ module.exports = {
'import/resolver': {
typescript: {},
},
jsdoc: {
mode: 'typescript',
},
},
};
4 changes: 4 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
!.*
.yarn
.yarnrc.yml
merged-packages
1 change: 1 addition & 0 deletions .prettierrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* @type {import('prettier').Options}
*/
module.exports = {
plugins: ['prettier-plugin-packagejson'],
// All of these are defaults except singleQuote, but we specify them
// for explicitness
quoteProps: 'as-needed',
Expand Down
20 changes: 9 additions & 11 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,10 @@
"changelog:update": "yarn workspaces foreach --no-private --parallel --interlaced --verbose run changelog:update",
"changelog:validate": "yarn workspaces foreach --no-private --parallel --interlaced --verbose run changelog:validate",
"create-package": "ts-node scripts/create-package",
"lint": "yarn lint:eslint && yarn lint:misc --check && yarn constraints && yarn lint:dependencies",
"lint:dependencies": "depcheck && yarn dedupe --check",
"lint:dependencies:fix": "depcheck && yarn dedupe",
"lint:eslint": "eslint . --cache --ext js,ts",
"lint:fix": "yarn lint:eslint --fix && yarn lint:misc --write && yarn constraints --fix && yarn lint:dependencies:fix",
"lint:misc": "prettier '**/*.json' '**/*.md' '!**/CHANGELOG.old.md' '**/*.yml' '!.yarnrc.yml' '!merged-packages/**' --ignore-path .gitignore",
"lint": "./scripts/lint.sh check",
"lint:fix": "./scripts/lint.sh fix",
"lint:fix:only-branch": "./scripts/lint.sh fix --relative-to-branch",
"lint:only-branch": "./scripts/lint.sh check --relative-to-branch",
"prepack": "./scripts/prepack.sh",
"prepare-preview-builds": "./scripts/prepare-preview-builds.sh",
"publish-previews": "yarn workspaces foreach --no-private --parallel --verbose run publish:preview",
Expand All @@ -39,7 +37,7 @@
"update-readme-content": "ts-node scripts/update-readme-content.ts"
},
"simple-git-hooks": {
"pre-push": "yarn lint"
"pre-push": "./scripts/pre-push.sh"
},
"resolutions": {
"tsup@^8.0.2": "patch:tsup@npm%3A8.0.2#./.yarn/patches/tsup-npm-8.0.2-86e40f68a7.patch"
Expand All @@ -64,23 +62,23 @@
"babel-jest": "^27.5.1",
"depcheck": "^1.4.7",
"eslint": "^8.44.0",
"eslint-config-prettier": "^8.5.0",
"eslint-config-prettier": "^9.1.0",
"eslint-import-resolver-typescript": "^2.5.0",
"eslint-interactive": "^10.8.0",
"eslint-plugin-import": "2.26.0",
"eslint-plugin-jest": "^27.1.5",
"eslint-plugin-jsdoc": "^39.9.1",
"eslint-plugin-n": "^15.7.0",
"eslint-plugin-prettier": "^4.2.1",
"eslint-plugin-prettier": "^5.1.3",
"eslint-plugin-promise": "^6.1.1",
"eth-block-tracker": "^8.0.0",
"execa": "^5.0.0",
"isomorphic-fetch": "^3.0.0",
"jest": "^27.5.1",
"jest-silent-reporter": "^0.5.0",
"nock": "^13.3.1",
"prettier": "^2.7.1",
"prettier-plugin-packagejson": "^2.4.5",
"prettier": "^3.2.5",
"prettier-plugin-packagejson": "^2.5.0",
"simple-git-hooks": "^2.8.0",
"ts-node": "^10.9.1",
"tsup": "^8.0.2",
Expand Down
268 changes: 268 additions & 0 deletions scripts/lint.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,268 @@
#!/bin/bash

set -euo pipefail

DEFAULT_BRANCH_NAME=main

already_printed_banner=0

red() {
printf "\x1B[31m"
echo -n "$@"
printf "\x1B[0m"
}

magenta() {
printf "\x1B[35m"
echo -n "$@"
printf "\x1B[0m"
}

bold() {
printf "\x1B[1m"
echo -n "$@"
printf "\x1B[22m"
}

banner() {
if [[ $already_printed_banner -eq 1 ]]; then
echo
fi
printf '%s\n' "$(magenta "===" "$@" "===")"
already_printed_banner=1
}

error() {
printf '%s\n' "$(red "ERROR:" "$@")"
}

print-usage() {
cat <<EOT
$(bold "SUMMARY")
Runs checks on files, or applies fixes to files that violate checks.
$(bold "USAGE")
$0 <mode> [--relative-to-branch]
$(bold "ARGUMENTS")
-r, --relative-to-branch Filters the set of files that will be
checked or fixed to only those which have
been added, changed, or deleted on this
branch relative to the base branch. If
omitted, all files will be processed.
<mode> How to process the files in this repo.
Whether to only run checks ($(bold "check")
or to fix files that violate checks ($(bold "fix")).
EOT
}

get-base-branch() {
local current_branch_name="$1"

# 1. Print the name of the refs attached to commits that have taken place on
# this branch.
# 2. Split the output so each line = one ref, excluding empty lines.
# 3. Exclude tags and remote branches.
# 4. Choose the first local branch name that is not the current branch name.
git log --pretty=format:'%D' | \
tr ',' '\n' | \
grep -v '^$' | \
sed -E 's/^[ ]+|[ ]+$//' | \
grep -E -v '^tag: ' | \
grep -E -v '^origin/' | \
grep -E -v '\b'$current_branch_name'\b' | \
head -n 1
}

get-files-to-lint() {
local current_branch_name="$1"

local base_branch
if [[ "$current_branch_name" == "$DEFAULT_BRANCH_NAME" ]]; then
base_branch="origin/$current_branch_name"
else
base_branch="$(get-base-branch "$current_branch_name")"
fi

if [[ -z "$base_branch" ]]; then
echo "<ERROR_NO_BASE_BRANCH>"
else
# List files in commits that have occurred on this branch
git diff "$base_branch...HEAD" --name-only
# List unstaged files
git diff --name-only
fi
}

get-unique-files-to-lint() {
get-files-to-lint "$@" | uniq
}

run-eslint() {
local mode="$1"
local relative_to_branch="$2"
local files_to_lint="$3"

local extra_eslint_options
if [[ "$mode" == "fix" ]]; then
extra_eslint_options="--fix"
else
extra_eslint_options=""
fi

set +e
if [[ $relative_to_branch -eq 1 ]]; then
echo "$files_to_lint" | while IFS=$'\n' read -r line; do
printf '%s\0' "$line"
done | grep --null-data -E '\.[jt]s$' | xargs -0 yarn eslint --cache $extra_eslint_options
else
yarn eslint --cache --ext js,ts $extra_eslint_options .
fi
set -e
}

run-prettier() {
local mode="$1"
local relative_to_branch="$2"
local files_to_lint="$3"

local extra_prettier_options
if [[ "$mode" == "fix" ]]; then
extra_prettier_options="--write"
else
extra_prettier_options="--check"
fi

set +e
if [[ $relative_to_branch -eq 1 ]]; then
echo "$files_to_lint" | while IFS=$'\n' read -r line; do
printf '%s\0' "$line"
done | xargs -0 yarn prettier --ignore-unknown $extra_prettier_options
else
yarn prettier $extra_prettier_options .
fi
set -e
}

run-yarn-constraints() {
local mode="$1"

local extra_yarn_constraints_options
if [[ "$mode" == "fix" ]]; then
extra_yarn_constraints_options="--fix"
else
extra_yarn_constraints_options=""
fi

set +e
yarn constraints $extra_yarn_constraints_options
set -e
}

run-yarn-depcheck() {
set +e
yarn depcheck
set -e
}

run-yarn-dedupe() {
local mode="$1"
local extra_yarn_dedupe_options

if [[ "$mode" == "fix" ]]; then
extra_yarn_dedupe_options=""
else
extra_yarn_dedupe_options="--check"
fi

set +e
yarn dedupe --check $extra_yarn_dedupe_options
set -e
}

main() {
local mode=
local relative_to_branch=0
local files_to_lint=""

while [[ $# -gt 0 ]]; do
case "${1:-}" in
-r | --relative-to-branch)
relative_to_branch=1
shift
;;
-*)
error "Unknown argument '$1'."
echo
print-usage
exit 1
;;
*)
if [[ -n $mode ]]; then
error "Unknown argument '$1'."
echo
print-usage
exit 1
else
mode="$1"
shift
fi
;;
esac
done

if [[ -z "$mode" ]]; then
error "Missing 'mode'."
echo
print-usage
exit 1
fi

local current_branch_name
current_branch_name="$(git branch --show-current)"

if [[ -z "$current_branch_name" ]]; then
error "Current branch not detected. Perhaps you're in detached HEAD state or in the middle of an operation?"
exit 1
fi

if [[ $relative_to_branch -eq 1 ]]; then
files_to_lint="$(get-unique-files-to-lint "$current_branch_name")"

if [[ "$files_to_lint" == "<ERROR_NO_BASE_BRANCH>" ]]; then
error "Could not find base branch."
exit 1
fi

if [[ -n "$files_to_lint" ]]; then
banner "Files to $mode"
echo "$files_to_lint" | while IFS=$'\n' read -r line; do
echo "- $line"
done
fi
fi

#if [[ $relative_to_branch -eq 1 ]]; then
#banner "Processing branch-specific files via ESLint"
#else
#banner "Processing all files via ESLint"
#fi
#run-eslint "$mode" "$relative_to_branch" "$files_to_lint"

#if [[ $relative_to_branch -eq 1 ]]; then
#banner "Processing branch-specific files via Prettier"
#else
#banner "Processing all files via Prettier"
#fi
#run-prettier "$mode" "$relative_to_branch" "$files_to_lint"

#banner "Processing Yarn constraints"
#run-yarn-constraints "$mode"

#banner "Processing dependencies"
#run-yarn-depcheck "$mode"
#run-yarn-dedupe "$mode"
}

main "$@"
10 changes: 10 additions & 0 deletions scripts/pre-push.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#!/bin/bash

# If the push refs start with (delete), then we're deleting a branch, so skip the pre-push hook
# Source: <https://github.com/typicode/husky/issues/169#issuecomment-1719263454>
stdin="$(cat -)"
if echo "$stdin" | grep -q "^(delete)"; then
exit 0
fi

exec yarn lint:only-branch
Loading

0 comments on commit f6072a8

Please sign in to comment.