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

Does not act on all files? (Despite lack of "filename: ..." and "exclude: ...") #4

Closed
moseb opened this issue Aug 9, 2019 · 3 comments · Fixed by #5
Closed

Does not act on all files? (Despite lack of "filename: ..." and "exclude: ...") #4

moseb opened this issue Aug 9, 2019 · 3 comments · Fixed by #5
Assignees
Labels
enhancement New feature or request

Comments

@moseb
Copy link
Contributor

moseb commented Aug 9, 2019

I have a weird problem here that maybe you have an idea about.
I'm tryint to find uses of mock where it should be unittest.mock for Python, instead.
So I have:

$ cat .relint.yml 
- name: "PyPI mock instead of Python 3 unittest.mock"
  pattern: "^(import mock)"
  hint: "Please use unittest.mock of Python 3 rather than plain/PyPI mock."

$ cat .pre-commit-config.yaml 
repos:
  - repo: https://github.com/codingjoe/relint
    rev: 1.0.0
    hooks:
      - id: relint

For config. Now it gets interesting:

$ git grep -E '^(import mock)' '*.py' | wc -l
72

$ pre-commit run --all-files | fgrep .py | wc -l
4

$ git ls-files '*.py' | xargs relint | fgrep .py | wc -l
4

So why are only 4 out of those 74 matches found by relint — any ideas?

@codingjoe codingjoe self-assigned this Aug 11, 2019
@codingjoe codingjoe added the question Further information is requested label Aug 11, 2019
@codingjoe
Copy link
Owner

@moseb thanks for reaching out.

I have an idea why your expression behaves that way. Relint matches the whole file, not line by line. This is the case, so people can write multi line expression.
The leading ^ will therefore only match cases where the import is the very first line.
If you limply make the pattern import mock, it should do what you expect.
Please let me know if that resolves your issue.

Best,
Joe

@moseb
Copy link
Contributor Author

moseb commented Aug 12, 2019

There is re.MULTILINE for the start of the string problem: https://docs.python.org/2/library/re.html#re.MULTILINE

When specified, the pattern character '^' matches at the beginning of the string and at the beginning of each line (immediately following each newline); and the pattern character '$' matches at the end of the string and at the end of each line (immediately preceding each newline). By default, '^' matches only at the beginning of the string, and '$' only at the end of the string and immediately before the newline (if any) at the end of the string.

Would adding re.MULTILINE be an option? The current behavior is rather unexpected and makes line based cases more difficult — (^|\n)import mock in my case. re.MULTILINE should allow both at the same time. What do you think?

@codingjoe
Copy link
Owner

@moseb that's a brilliant idea! You would add the flag in this line:

pattern=re.compile(test['pattern']),

I would love it, would you conjure up a pull-request.

@codingjoe codingjoe reopened this Aug 14, 2019
@codingjoe codingjoe added enhancement New feature or request and removed question Further information is requested labels Aug 14, 2019
moseb added a commit to moseb/relint that referenced this issue Aug 15, 2019
As a result, in a pattern ^ and $ are now matching start and end
of the line, not just start and end of the string.
codingjoe pushed a commit that referenced this issue Aug 16, 2019
Compile patterns with the `re.MULTILINE` flag. As a result,
in a pattern ^ and $ are now matching start and end
of the line, not just start and end of the string.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants