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

Providing an invalid file path should raise instead of silently ignoring it #388

Open
jwoertink opened this issue Jul 11, 2023 · 7 comments
Labels

Comments

@jwoertink
Copy link

Sort of related: #362

But in this case, if you just tell ameba to check a file that doesn't exist, it returns Inspecting 0 files

❯ ./bin/ameba sdfawsdf
Inspecting 0 files



Finished in 10 microseconds
0 inspected, 0 failures

I'm thinking of maybe a situation where someone has a CI and types the name of a path wrong, the CI runs and says things are all good, but you don't find out until later that's not the case.

@Sija Sija added the bug label Jul 11, 2023
@Sija
Copy link
Member

Sija commented Jul 13, 2023

Duplicate of #362

@Sija Sija marked this as a duplicate of #362 Jul 13, 2023
@Sija Sija closed this as not planned Won't fix, can't repro, duplicate, stale Jul 13, 2023
@veelenga
Copy link
Member

@Sija this looks like a separate issue. See the difference between config file path and a file to check.

@jwoertink
Copy link
Author

Yeah, I originally considered adding to the other issue, but thought maybe they'd be different enough where one is

./bin/ameba -c not_there.yml

and this is

./bin/ameba not_there.cr

I guess another option could be to combine them in to a single issue that's like "passing file paths that don't exist should raise an error in all cases"?

@Sija Sija reopened this Jul 13, 2023
@Sija
Copy link
Member

Sija commented Jul 13, 2023

You're right, I was trigger happy 😅

@stufro
Copy link
Contributor

stufro commented Jul 26, 2023

This is an interesting one to implement because the command line arguments can be a single file or a glob. In glob_utils.cr#expand the list of arguments are expanded and only file paths which exist are returned from Dir[glob].

This makes it hard to catch non-existent files anywhere else in the code base after the globs have been expanded, which is why I'm thinking to implement this in GlobUtils#expand.

My idea is to raise an exception if a particular glob doesn't return any results from Dir[glob]. This would raise in both the below examples:

> ./bin/ameba not_there.cr
Error: No files found matching not_there.cr

> ./bin/ameba not_there/**/*.cr
Error: No files found matching not_there/**/*.cr

I'll make a PR using this approach, but I'm open to feedback on different approaches.

@veelenga
Copy link
Member

@stufro thanks for taking a look.

This is what I would expect, however, need to understand if there are non-intuitive edge cases.

@stufro
Copy link
Contributor

stufro commented Jul 26, 2023

Thanks - #394. I've tried all the edge cases I can think of and it work as I would expect.

@Sija Sija reopened this Dec 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants