Cfn-python-lint: Support checking multiple files at once

[alexjurkiewicz]

Please support an arbitrary number of files passed in on the commandline. For example:

cfn-lint template1.yaml template2.yaml

[asottile]

I started poking at this in https://github.com/awslabs/cfn-python-lint/pull/152

Here's a patch that gets closer (on top of that branch):

diff --git a/src/cfnlint/core.py b/src/cfnlint/core.py
index df383ac..2b10743 100644
--- a/src/cfnlint/core.py
+++ b/src/cfnlint/core.py
@@ -93,9 +93,11 @@ def create_parser():

     # Alllow the template to be passes as an optional or a positional argument
     parser.add_argument(
-        'template', nargs='?', help='The CloudFormation template to be linted')
+        'templates', nargs='*', default=[], help='The CloudFormation template to be linted')
     parser.add_argument(
-        '-t', '--template', dest='template_alt', help='The CloudFormation template to be linted')
+        '-t', '--template', dest='templates', action='append',
+        help='The CloudFormation template to be linted'
+    )

     parser.add_argument(
         '-b', '--ignore-bad-template', help='Ignore failures with Bad template',
@@ -185,14 +187,7 @@ def get_template_args_rules(cli_args):
     fmt = args.format
     formatter = get_formatter(fmt)

-    # Filename can be speficied as positional or optional argument. Positional
-    # is leading
-    if args.template:
-        filename = args.template
-    elif args.template_alt:
-        filename = args.template_alt
-    else:
-        filename = None
+    filenames = args.templates

     if filename:
         ignore_bad_template = args.ignore_bad_template

[fatbasstard]

Personally I'm nog a big fan of handling multiple files in the linter...

The linter should focus on linting a template. Adding a wrapper layer to work/process multiple files only add complexity and possible errors. Apart from the fact that you don’t know how you want the output then.

If you want to check multiple templates, that’s really no issue to build that outside of the linter.
Create a bash script that loops over a folder of parses multiple files and call the linter.
Or implement the Python module in your own codebase and handle the multiple file logic there.

[cmmeyer]

I'm with @fatbasstard on this one -- I worry the output will get complex for multiple files and move into land they may make IDE integration more complex. But not reason you couldn't write a "batch" wrapper script using the core libraries just like we do for the IDE wrappers.

[asottile]

Nearly every linter supports as a standard interface linter [filename, [filename ...]] (except this one) please consider the precedents set by tools like pyflakes / flake8 / pylint / eslint / prettier / black / etc.

Not only does this make invocation dead simple with xargs, it is much more performant (the interpreter and subprocess overhead occurs once instead of a linear amount of times by file). This overhead is especially high on windows. (even on Linux, compare the time taken between git ls-files -- '*.py' | xargs pylint vs. git ls-files -- '*.py' | xargs -n1 pylint).

Also in line with other posix tools you could simply invoke with a glob linter *.yaml instead of needing to copy paste a complicated wrapper everywhere.

The main motivation here is so that this linter can easily be integrated into other frameworks, in particular https://pre-commit.com

[alexjurkiewicz]

Yep, when I'm linting my CFN files I can run yaml-lint $files but need a special case for file in $files ; do cfn-lint $file which is annoying.

[asottile]

(just to put numbers to what I tapped out on my phone above re: pylint):

$ uname -a
Darwin Macbook-Pro 17.6.0 Darwin Kernel Version 17.6.0: Tue May  8 15:22:16 PDT 2018; root:xnu-4570.61.1~1/RELEASE_X86_64 x86_64 i386 MacBookPro11,4 Darwin
$ git ls-files -- '*.py' | grep ^src/cfnlint | time xargs .tox/pylint36/bin/pylint 
...
       11.92 real        11.59 user         0.31 sys
$ git ls-files -- '*.py' | grep ^src/cfnlint | time xargs -n1 .tox/pylint36/bin/pylint
...
       75.78 real        69.26 user         5.93 sys

[alexjurkiewicz]

As per #166 and #172, a proposal for how to handle migration from single-template to multi-template support:

"Next" version:

1. Add --templates, which accepts multiple template files (comma-separated) and can be specified multiple times (eg --templates test1.yml,test2.json --templates test3.yaml)
2. Change -t to mean --templates
3. Change template position argument to allow zero or more (*)
4. Deprecate --template (suppress display in usage, and print warning to stderr if it's used)
5. When only one file is specified, output format is unchanged from not (except the warning is printed if applicable)
6. When multiple files are specified, output format can change if required

"Next+1" version:

1. Remove --template

[kddejong]

@alexjurkiewicz We had talked about (-r, -i, -a) being switched to append (argparse) or comma delimited list. I don't see that above is that still part of this?

[alexjurkiewicz]

I think that can be done as a separate PR, so I'll do that. Are you worried about backwards compatibility for those options with multiple items?

[hoshsadiq]

This would be cool, as this could potentially allow validating outputs when using nested stacks.

[adamchainz]

I'm +1 on multiple files being supported, it would make linting much faster. Currently a new Python process has to be spawned and all the code imported for each, a great overhead. Most linters I know of support multiple files.

[cmmeyer]

Just merged #331 which implements this enhancement!

[kddejong]

Released in v0.7.4