Handle unsupported code/dependencies files when FawltyDeps run with explicit list of files

[mknorps]

Problem statement

When FawltyDeps run with explicit list of files (as it is for pre-commit hooks, see #499) it raises an error for unsupported formats of code or dependencies files.

Solution ideas

1. Instead of raising error when FD traverse mechanism encounters an unparsable file, we could emit a warning or just skip the non-matching file.
2. For explicitly given files we could add a step to sort them to ones that may contain dependency declarations and code.

Explanation: Settings object

Run FawltyDeps on FawltyDeps with printing the settings object build from input arguments.

Without explicit list of files

fawltydeps --generate-toml-config

# Copy this TOML section into your pyproject.toml to configure FawltyDeps
# (default values are commented)
[tool.fawltydeps]
# actions = ['check_undeclared', 'check_unused']
# output_format = 'human_summary'
code = ['fawltydeps']
deps = ['pyproject.toml']
# pyenvs = ['.']
# ignore_undeclared = []
ignore_unused = ['actionlint-py', 'codespell', 'hypothesis', 'mkdocs', 'mkdocs-material', 'mypy', 'nox', 'poetry-plugin-export', 'pytest', 'ruff', 'types-PyYAML', 'uv']
...

With explicit list of files

fawltydeps pyproject.toml fawltydeps --generate-toml-config

# Copy this TOML section into your pyproject.toml to configure FawltyDeps
# (default values are commented)
[tool.fawltydeps]
# actions = ['check_undeclared', 'check_unused']
# output_format = 'human_summary'
code = ['fawltydeps', 'pyproject.toml']
deps = ['fawltydeps', 'pyproject.toml']
pyenvs = ['fawltydeps', 'pyproject.toml']
# ignore_undeclared = []
ignore_unused = ['actionlint-py', 'codespell', 'hypothesis', 'mkdocs', 'mkdocs-material', 'mypy', 'nox', 'poetry-plugin-export', 'pytest', 'ruff', 'types-PyYAML', 'uv']
...

BUT running FawltyDeps with this command:

fawltydeps pyproject.toml fawltydeps   

results in:

fawltydeps: error: Supported formats are .py and .ipynb; Cannot parse code: pyproject.toml

[jherland]

Thanks for digging into this. I think both or your suggestions are good, and I'm not immediately sure which to prefer...

The core of the issue seems to be that paths that end up on the FD command line (without --code, --deps, or --pyenv) will end up being interpreted as if they were passed to all three. And when one of those options does not like the (file) path it is given, then we fail and abort, rather than continue with a warning.

This logic is currently (and unhelpfully IMHO) split across two different parts of the code:

1. In Settings.create(), we copy search_paths (paths given as positional args) into settings.code/.deps/.pyenvs (unless these are also explicitly set).
2. In find_sources(), we traverse each of settings.code/.deps/.pyenvs handling paths as if they were intentionally passed to these respective options. When these paths come from search_paths in (1) -- and are thus passed with less explicit intention -- then we get these unfortunate consequences:
   • For .code, directories are added to the traversal (good), but files are automatically converted into CodeSource objects, which fails immediately for non-code files when the CodeSource constructor checks the filename suffix (bad).
   • For .deps, directories are added to the traversal (good), but files that don't match our PARSER_CHOICES will raise an UnparseablePathError (bad for any file that is not a deps file).
   • For .pyenvs, directories are either Python environments directly, or added to the traversal (good), but any file raises an immediate UnParseablePathError (bad).

So, yes, we definitely need to do something about files that are passed as positional arguments on the command line!

[jherland]

I think I'm starting to prefer your second solution (pre-sorting) over your first (printing a warning).

It boils down to this: I think there is a significant difference between fawltydeps pyproject.toml fawltydeps and fawltydeps --code=pyproject.toml fawltydeps: The first command is reasonable (at least when we see how fawltydeps is being called from pre-commit), and we should do our best to do the Right Thing™️. The second command is an obvious error where the user is asking us to parse pyproject.toml as if it was Python code.

Printing a warning for pyproject.toml is IMHO the wrong solution in both of these cases: In the first case, the warning would be a distraction, and we should simply "do better" to infer that the user never meant to parse pyproject.toml as code. In the second case, the user's intention was for us to do something impossible, so I think error + abort is a better (more honest) result than warning + continue.

Pre-sorting files into code and/or declarations (remember, files like setup.py can be both) is probably the better way to go. Although there will be some added logic to do this pre-sort, we can reuse much code that is already there:

• CodeSource() raising UnparseablePathError lets us know that a file is not a code source.
• first_applicable_parser() returning None lets us know that a file is not a deps source.

In fact, I think we can build this into each of those three for loops in find_sources() (starting at line 106 in traverse_project.py): If any single path raises an UnparseablePathError, AND that path came from search_paths (i.e. was not explicitly passed to --code/--deps/--pyenv), then we should skip that path, probably with an INFO message(?).¹

In order for this to work, we need find_sources() to know whether a path was passed via search_paths, or explicitly set in .code/.deps/.pyenvs, That means that we should probably move the search_paths logic out of Settings.create() and into a new Settings object property, so that this information ("is this path a positional arg, or was it explicitly passed to an option?") can be accessed from traverse_project.

Does this make sense?

Footnotes

1. If the path was actually explicitly/intentionally passed to --code/--deps/--pyenv (e.g. --code=pyproject.toml, --deps=main.py, or --pyenv=pyproject.toml), then I do want the UnparseablePathError to be propagated, to trigger the more honest error + abort I showed above. ↩

[jherland]

There is another factor that is related to this issue, but I don't think it affects the above analysis:

As part of #466, we need to start to parse every Python source file for dependency declarations. (Or rather, if we find PEP 723 inline script metadata, we need to run a separate analysis where we match code and deps for that file only, separately from the rest of the project.)

On some level, this means that every code file is now also a potential deps file. In practice, the way I foresee us solving this is that we introduce a new Source subtype (e.g. ScriptSource or similar), which signifies that this source should be analyzed differently from other CodeSource objects.

In practice I don't think this changes how we should handle files passed as positional args on the command line, so please feel free to ignore this side quest. 😅