Lint for parameters being used in actions to which they don't apply

[kichristensen]

What does this change

Update linter to check if parameters are being used in actions to which they don't apply, as it would cause a runtime failure.

If a parameter is used in an action to which is doesn't apply porter lint will produce the output similar to this:

error(porter-101) - Parameter does not apply to action
install: 1st step in the exec mixin (Install Hello World)
Parameter onlyAppliesToUninstall does not apply to install action
See https://porter.sh/docs/references/linter/#porter-101 for more information
---

What issue does it fix

Closes #1018

Notes for the reviewer

Put any questions or notes for the reviewer here.

Checklist

• Did you write tests?
• Did you write documentation?
• Did you change porter.yaml or a storage document record? Update the corresponding schema file.
• If this is your first pull request, please add your name to the bottom of our Contributors list. Thank you for making Porter better! 🙇‍♀️

Reviewer Checklist

• Comment with /azp run test-porter-release if a magefile or build script was modified
• Comment with /azp run porter-integration if it's a non-trivial PR

[schristoff]

My initial thought is: "Cool, but are we breaking backward compat with this?" - which I think is a a big yes, but I think it may be okay to do it in this case.

[kichristensen]

@schristoff Completely forgot about backwards compability with this change. That said, in my opinion, it is not fully backwards compatible.
But as is causes a hard runtime error instead for failing on during build, I think it is acceptable, and even better to fail early.
It is a grey zone 🤷

No matter what happens with this PR, there should probably be some sort of strategy for adding new lints, as all new lints can be considered breaking changes in the sense that they potentially breaks existing builds, at least for errors. Might be worth taking up on a community meeting.

[sgettys]

I agree it's better to hit this at build time than runtime. Any breakages because should only be on already broken bundles I would imagine

[sgettys]

This should be a defer otherwise this won't get closed if the return on 208 is hit. Any reason to have 2 different spans in here instead of a single one?

[kichristensen]

You are correct, in both cases. Also I cannot find any good reasons for 2 spans here. I will change it

[sgettys]

Can you add an integration test for this as well?

[kichristensen]

Can you add an integration test for this as well?

@sgettys Sure, it has been added

[schristoff]

Since our next release is 1.1.0 (I know, breaking compat should be 2.0+ but this is a small compat thing) I'm fine with merging this sooner rather than later.

[schristoff]

Looks good once conflicts are resolved