realm / SwiftLint

A tool to enforce Swift style and conventions.
https://realm.github.io/SwiftLint
MIT License
18.65k stars 2.22k forks source link

is possible to check only for "git diff" #413

Closed steve21124 closed 8 years ago

steve21124 commented 8 years ago

Is it possible to check only for git diff section?

here is the scenario, there are several developers checkin the code, I would like to check which developer that violate the rules and send email to that developer

daniel-beard commented 8 years ago

For my projects, I use a script to cull out the violations, as swiftlint is fast enough to lint the entire code relatively quickly. Here's an example script that I use: https://gist.github.com/daniel-beard/db4059e5a351a12060a8

larslockefeer commented 8 years ago

Yes this is possible:

git diff --cached --name-only | while read filename; do
    swiftlint lint --path "$filename";
done

You could even add SwiftLint as a pre-commit hook, such that you cannot commit (without adding a skip option to your commit) without all changed files passing SwiftLint.

steve21124 commented 8 years ago

thanks. good input

mgrebenets commented 8 years ago

This works for staged changes only, but not for modified files. Dropping the --cached part will give you list of modified files. It also needs to filter out non Swift files. Here's the script I ended up using:

# Run SwiftLint
START_DATE=$(date +"%s")

SWIFT_LINT=/usr/local/bin/swiftlint

# Run SwiftLint for given filename
run_swiftlint() {
    local filename="${1}"
    if [[ "${filename##*.}" == "swift" ]]; then
        ${SWIFT_LINT} autocorrect --path "${filename}"
        ${SWIFT_LINT} lint --path "${filename}"
    fi
}

if [[ -e "${SWIFT_LINT}" ]]; then
    echo "SwiftLint version: $(${SWIFT_LINT} version)"
    # Run for both staged and unstaged files
    git diff --name-only | while read filename; do run_swiftlint "${filename}"; done
    git diff --cached --name-only | while read filename; do run_swiftlint "${filename}"; done
else
    echo "${SWIFT_LINT} is not installed."
    exit 0
fi

END_DATE=$(date +"%s")

DIFF=$(($END_DATE - $START_DATE))
echo "SwiftLint took $(($DIFF / 60)) minutes and $(($DIFF % 60)) seconds to complete."

We put is somewhere in the project, e.g. in Scripts/swiftlint.sh and then in Xcode itself the build phase step is quite short "${SRCROOT}/Scripts/swiftlint.sh". I don't like looking at Xcode project diffs much :)

lukas1 commented 7 years ago

you could replace this check if [[ "${filename##*.}" == "swift" ]]; then by git diff --name-only | grep "\.swift" when doing the while loop to only loop on swift files

mazen-kasser commented 7 years ago

@mgrebenets Hi Max, its great to find your answer :) What is the use of local filename="${1}"? it will work even without this line.

danilobecke commented 7 years ago

hi, I made a bash script to check only modified files https://github.com/danilobecke/lint/blob/master/lintDiffOnly.sh

jpsim commented 7 years ago

Assuming SwiftLint's cache is working reliably, what's the advantage of doing this?

marcelofabri commented 7 years ago

Assuming SwiftLint's cache is working reliably, what's the advantage of doing this?

One use case I can see would be slowly integrating SwiftLint by requiring only touched files to be without warnings. This way you can avoid a huge reformatting commit for example.

jpsim commented 7 years ago

That's a great use case

mrvincenzo commented 6 years ago

I suggest to use git diff with --diff-filter=d to filter out delete files, i.e.

git diff --cached --diff-filter=d --name-only

or

git diff --diff-filter=d --name-only

Calling SwiftLint on deleted file will result in an error

abdelmagied94 commented 5 years ago

I think this doesn't take into consideration the excluded list in the configuration file.

mgrebenets commented 5 years ago

@abdelmagied94 That's the case, in our full version we have something like this:

# Run code checks for given filename.
run_checks() {
    local filename="${1}"

    # Ignore Pods and Vendor paths.
    [[ ${filename} =~ Pods/ ]] && return 0
    [[ ${filename} =~ Vendor/ ]] && return 0

    # Ignore autogenerated SwiftGen files.
    [[ ${filename} =~ .*SwiftGen\.swift ]] && return 0

    # Ignore templates and generated files.
    [[ ${filename} =~ .*Generated.* ]] && return 0

    # Only lint .swift files.
    ! [[ ${filename##*.} == "swift" ]] && return 0

    # Run the checks.
    ${SWIFTLINT} autocorrect --path "${filename}"
    ${SWIFTFORMAT} "${filename}"
    ${SWIFTLINT} lint --strict --path "${filename}"
}

The downside is that we have to duplicate the logic of SwiftLint and in this case SwiftFormat exclude list, but works for us given that we have those exclude lists following some naming conventions.

It could be improve by parsing SwiftLint yaml config in some way and matching file names against it, if it's worth the additional effort.

george-lim commented 4 years ago

Made a gist for accomplishing exactly this. It filters out violations that are not in the git diff hunks, and hooks via a bash run script. https://gist.github.com/george-lim/f1166bc75d9047495934a6eb974bb127

RohithSriram commented 4 years ago

Is there a way to check SwiftLint warnings and errors only for the lines that were modified and not the files? Because our code base is pretty big with some files spanning over 3,000 lines. So people who are making changes to just a couple of lines in that file end up getting warnings and errors for the complete file.

lukas1 commented 4 years ago

@RohithSriram Not an answer to your problem, so sorry about that, but if your file is over 3000 lines, then that's a problem and you should be thinking about how to make those files smaller.

Separate the functionality into multiple smaller components.

Large files like that are hard to maintain, especially a new developer to the project will have no idea what is going on in that file.

jpsim commented 3 years ago

FYI I'm considering adding a "stable git revision" CLI option to SwiftLint and I'd be curious to hear your thoughts: #3536

adrianod1as commented 3 years ago

I tried the scripts, but it seems that by running swiftlint individually on each file (while loop), it avoids the exit.

The errors are still shown but the build succeeds, which I would like to prevent.

Captura de Tela 2021-06-17 às 17 31 15

Script:

# Run SwiftLint for given filename using modules config
run_swiftlint() {
    local filename="${1}"
    if [[ "${filename##*.}" == "swift" ]]; then
        swiftlint --config ../../.swiftlint-modules.yml --path "${filename}"
    fi
}

# Retrieve edited/created files
git diff origin/dev --name-only --relative | while read filename; do run_swiftlint "${filename}"; done

If I run only

swiftlint --config ../../.swiftlint-modules.yml

it works as expected. That's why I am suspecting that "individually" is the problem.

Any insights?

jpsim commented 3 years ago

@adrianod1as can you take a look at #3536 and let me know if it’s a good solution for you?

Alexi-K commented 3 years ago

If I run only

swiftlint --config ../../.swiftlint-modules.yml

it works as expected. That's why I am suspecting that "individually" is the problem.

Any insights?

Perhaps you've placed script phase run in a wrong order for linter

DanijelHuis commented 2 years ago

@adrianod1as I guess when you execute single command it passes the exit code of that command as exit code of the whole script, that is why it works for single lint command but not for a loop - not sure about that, just guessing.

I added hasErrors variable to track if error occurred while linting and then returned it at the end of the script. Now if lint finds errors the build will fail. Note that in modified script I run "swiftlint lint..." only once (I don't run it with autocorrect). Also I included new files and excluded deleted files from the lint.


# Variables
START_DATE=$(date +"%s")
SWIFT_LINT=/usr/local/bin/swiftlint
hasErrors=0

# Run SwiftLint for given filename
run_swiftlint() {
    local filename="${1}"
    if [[ "${filename##*.}" == "swift" ]]; then
        ${SWIFT_LINT} lint --path "${filename}"
        if [[ $? != 0 ]]; then
            hasErrors=1
        fi
    fi
}

if [[ -e "${SWIFT_LINT}" ]]; then
    echo "SwiftLint version: $(${SWIFT_LINT} version)"
    # Run for staged files
    for filename in $(git diff --diff-filter=d --name-only);
    do
        run_swiftlint "${filename}";
    done

    # Run for unstaged files
    for filename in $(git diff --cached --diff-filter=d --name-only);
    do
        run_swiftlint "${filename}";
    done

    # Run for added files
    for filename in $(git ls-files --others --exclude-standard);
    do
        run_swiftlint "${filename}";
    done
else
    echo "${SWIFT_LINT} is not installed."
    exit 0
fi

END_DATE=$(date +"%s")
DIFF=$(($END_DATE - $START_DATE))
echo "SwiftLint took $(($DIFF / 60)) minutes and $(($DIFF % 60)) seconds to complete."
exit $hasErrors```
anoop4real commented 7 months ago

@DanijelHuis Tried to use your script but I ran into problems

  1. Many of my folders had spaces and the script broke.
  2. Even after fixing the spaces I was getting "no lintable files found at paths", then had to add relative paths $(git rev-parse --show-toplevel)/${filename}

did some modifications, do let me know if there is a better solution

 # Variables
START_DATE=$(date +"%s")
SWIFT_LINT="/usr/local/bin/swiftlint"
hasErrors=0

# Run SwiftLint for given filename
run_swiftlint() {
    local filename="${1}"
    if [[ "${filename##*.}" == "swift" ]]; then
        "${SWIFT_LINT}" lint "$(git rev-parse --show-toplevel)/${filename}"
        if [[ $? != 0 ]]; then
            hasErrors=1
        fi
    fi
}

if [[ -e "${SWIFT_LINT}" ]]; then
    echo "SwiftLint version: $("${SWIFT_LINT}" version)"
    # Run for staged files
    git diff --diff-filter=d --name-only | while IFS= read -r filename; do
        run_swiftlint "${filename}"
    done

    # Run for unstaged files
    git diff --cached --diff-filter=d --name-only | while IFS= read -r filename; do
        run_swiftlint "${filename}"
    done

    # Run for added files
    git ls-files --others --exclude-standard | while IFS= read -r filename; do
        run_swiftlint "${filename}"
    done
else
    echo "${SWIFT_LINT} is not installed."
    exit 0
fi

END_DATE=$(date +"%s")
DIFF=$(($END_DATE - $START_DATE))
echo "SwiftLint took $(($DIFF / 60)) minutes and $(($DIFF % 60)) seconds to complete."
exit $hasErrors
harrison-phonehub commented 4 months ago

@anoop4real I implemented your script.

However, the variable hasErrors seems to reset.

When putting echo hasErrors after hasErrors=1 you can see that it is incremented to 1 when swiftlint catches failures, but the exit $hasErrors at the end will always exit with code 0