Is "Module Inspection" misleading?

[wata727]

Recently, I've come to think that Module Inspection is a feature that is easily misunderstood by its name.

This feature does inspect child modules, but discards all but the issues associated with the module call arguments. This is because if it reports all the issues with the child modules, there is no other way to ignore the issue with the third-party modules than to ignore the whole module. Strictly speaking, this behavior appears to be inspecting "Module Call", not "Module".

On the other hand, for users who want to inspect multiple local modules together, "Module Inspection" looks like an option to do just that.

If this hypothesis is right, we may need to change the terminology, option names, etc.

[bendrucker]

The smallest change I can think of here would be to refer to "Module Attribute Inspection." It's hard to understand the nuance here without reading the docs, but maybe that would be enough to force users to do that rather than assume it has something to do with recursion.

[domos4]

Hey, thanks for this great tool, I find it super helpful!

From my perspective, Module Inspection feature is very confusing, I understood how it works only after reading this issue. Reading the documentation didn't help, I spent quite a while scratching my head on why running the whole project with --recursive flag checks shared local modules and running tflint in a single module, that calls a child local module with --module flag doesn't lint this shared local module.

I suggest rephrasing the --module flag description and the docs to make it clear, that module inspection checks only child module calls, not the actual child modules.
Ideally, --module flag should make tflint inspect all the local modules that were used in a given module, may I ask whether it doesn't work like that by design or it's just a missing feature? Not having it work like that makes my CI flow much more complex than when working with other linters.

Thanks for your work, it's highly appreciated!

[wata727]

@domos4 Thank you for your feedback. Do you think your confusion is resolved by renaming the feature to "Module Call Inspection" and changing the flag to --module-call?

Also, although this behavior is documented here, do you think there is something missing or a navigation issue?

Ideally, --module flag should make tflint inspect all the local modules that were used in a given module, may I ask whether it doesn't work like that by design or it's just a missing feature? Not having it work like that makes my CI flow much more complex than when working with other linters.

First, in terms of local module "call" inspection, this is a missing feature. This is tracked in #1502. On the other hand, it's a design issue whether the local module "definition" should be inspected.

When using the --recursive flag, the local module "definitions" are automatically included in the inspection target. Below is an example:

├── main.tf
└── modules
    └── server
        └── main.tf

module "server" {
  source = "./modules/server"
}

In this case, if tflint --module automatically inspects against the module "definition" in ./modules/server, the issues will be reported twice.

Given this background, I now think that in principle a design that inspects only the current module might be simpler and easier to understand. What do you think?

[bendrucker]

I'm -1 on following any modules and inspecting them as root modules. Terraform doesn't understand the concept of a repository. Even though most local module calls will source something like ./modules/foo, that's not a requirement. Ancestor paths (..) and absolute paths are two obvious potential edge cases. Adding this magic would seem to invite the need to customize it.

+1 to "Module Call" (vs "Module"), but not entirely convinced that it's worth the breaking change to the CLI and config to do that across the board. But a good distinction for docs and help text.

[wata727]

Totally agree. Renaming will probably help prevent misunderstandings, but I'm wondering if the --module-call flag immediately makes us imagine what it actually does. There may be other good alternatives.

[wata727]

I'm working on this issue in addition to fixing #1502. This fix requires a breaking change to the --module flag, so now is a chance to make this change.

Currently, I'm positive about naming this feature "Calling Modules". This naming allows for a clear distinction between inspecting a child module directly as the root module, and to indicate the possibility of additional inspection to module blocks (module calls) in the root module.