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

Prevent sub-folder files from being analyzed twice #124

Merged
merged 2 commits into from
Sep 2, 2020
Merged

Prevent sub-folder files from being analyzed twice #124

merged 2 commits into from
Sep 2, 2020

Conversation

paulo-ferraz-oliveira
Copy link
Collaborator

This is what is happening to me...
I have an elvis.config file such as

[{elvis, [
    {config, [
        #{ dirs => ["src",
                    "src/sub"],
           filter => "*.erl",
           ruleset => erl_files
    ]},
    {verbose, true}
]}].

When analyzing file file.erl inside folder src/sub it gets analyzed twice, since elvis_core's internals match it with both src/ and src/sub/.
The "solution" found was to un-duplicate these occurrences.

@elbrujohalcon
Copy link
Member

Does this fix inaka/elvis#508?

@elbrujohalcon
Copy link
Member

Related: 1d80b28#r36143723

@paulo-ferraz-oliveira
Copy link
Collaborator Author

Does this fix inaka/elvis#508?

Let me try to check that tonight. If it doesn't I might want to tackle that also.

@paulo-ferraz-oliveira
Copy link
Collaborator Author

Regarding dirs:

  • ["src/**"]: same result (i.e. not fixed),
  • ["src", "src/**"]: fixed,
  • ["src", "src/*", "src/*/*"]: fixed.

@paulo-ferraz-oliveira
Copy link
Collaborator Author

I'm looking into why the issue occurs with ["src/**], but I need to understand initial requirements:

  1. is declaring ["src"] supposed to be recursive?
  2. is declaring ["src/**"] supposed to be recursive?

I feel there's a simpler approach to the problem (e.g. filesystem folder tree traversal...).

@paulo-ferraz-oliveira
Copy link
Collaborator Author

I'm looking into why the issue occurs with ["src/**], but I need to understand initial requirements:

  1. is declaring ["src"] supposed to be recursive?
  2. is declaring ["src/**"] supposed to be recursive?

I feel there's a simpler approach to the problem (e.g. filesystem folder tree traversal...).

I think I found my answer here: https://erlang.org/doc/man/filelib.html#wildcard-1.

If we don't remove ** in the comparison bit stuff like src/**/*.erl might happen, which will
not include src/m1.erl, for example, as expected (if `dirs` is "src/**"
@@ -128,6 +130,9 @@ replace_stars(Glob) -> re:replace(Glob, "[[][*][]]", ".*", [global]).

replace_questions(Glob) -> re:replace(Glob, "[[][?][]]", ".", [global]).

reduce_stars(DirAndFilter) ->
re:replace(DirAndFilter, "/\\*+/", "/", [global, {return, list}]).
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This improves the analysis scope. Specifically, it deals with all the issues in inaka/elvis#508.

@elbrujohalcon
Copy link
Member

Thank you, @paulo-ferraz-oliveira !!!

@paulo-ferraz-oliveira
Copy link
Collaborator Author

Thank you, @paulo-ferraz-oliveira !!!

Oh, this is nice 👍

@paulo-ferraz-oliveira paulo-ferraz-oliveira deleted the fix/double_file_analysis branch September 2, 2020 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants