-
Notifications
You must be signed in to change notification settings - Fork 9
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
feat: implements the container_value
rule
#142
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
claymcleod
force-pushed
the
feat/container-tags
branch
5 times, most recently
from
July 26, 2024 04:38
ed91471
to
46840ec
Compare
claymcleod
commented
Jul 26, 2024
claymcleod
commented
Jul 26, 2024
claymcleod
commented
Jul 26, 2024
claymcleod
commented
Jul 26, 2024
claymcleod
force-pushed
the
feat/container-tags
branch
2 times, most recently
from
July 26, 2024 14:06
a04faec
to
57f7a5f
Compare
claymcleod
commented
Jul 26, 2024
claymcleod
force-pushed
the
feat/container-tags
branch
from
July 26, 2024 14:10
57f7a5f
to
8fbe20a
Compare
claymcleod
commented
Jul 26, 2024
claymcleod
commented
Jul 26, 2024
claymcleod
force-pushed
the
feat/container-tags
branch
2 times, most recently
from
July 26, 2024 14:18
478ec47
to
ae11841
Compare
claymcleod
commented
Jul 26, 2024
claymcleod
force-pushed
the
feat/container-tags
branch
2 times, most recently
from
July 26, 2024 14:27
21955bb
to
6b6b4c1
Compare
claymcleod
commented
Jul 26, 2024
claymcleod
force-pushed
the
feat/container-tags
branch
2 times, most recently
from
July 26, 2024 14:36
00bae84
to
4b70d97
Compare
adthrasher
reviewed
Jul 29, 2024
adthrasher
reviewed
Jul 29, 2024
adthrasher
reviewed
Jul 29, 2024
a-frantz
reviewed
Jul 31, 2024
claymcleod
force-pushed
the
feat/container-tags
branch
15 times, most recently
from
August 1, 2024 22:38
ce7c791
to
427f06b
Compare
claymcleod
commented
Aug 1, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be ready for review again.
adthrasher
approved these changes
Aug 2, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, pending @a-frantz's comments.
claymcleod
force-pushed
the
feat/container-tags
branch
from
August 5, 2024 14:08
427f06b
to
e5714f0
Compare
a-frantz
approved these changes
Aug 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request adds a new rule to
wdl-lint
.ContainerValue
This rule is intended to implement the
mutable_container
andimmutable_container_not_tagged
baseline lint rules (link). In so doing, it was an exploration in implementing various wrappers within the AST that were built from existing AST nodes (rather than cast fromSyntaxNode
s). I also added in a few checks related to cointainers that seemed logical to me, namely:container
arrays with zero elements are ambiguous and should be replaced with something concrete.container
arrays with one element should be replaced with a single, literal string.container
arrays with the special 'any' container designator (*
) are ambiguous, as you're both indicating that a set of specific containers should be used and that any container can be used. As such, it should be avoided.Additionally, I fixed a small bug ("oversight"?) I discovered in the
MissingRuntime
lint while developing this:MissingRuntime
should only be flagging things for WDL v1.1 or earlier, as theruntime
section was deprecated in favor ofrequirements
in WDL v1.2.Before submitting this PR, please make sure:
CHANGELOG.md
(see"keep a changelog" for more information).
Rule specific checks:
RULES.md
.rules()
function inwdl-lint/src/lib.rs
.wdl-lint/tests/lints
that covers everypossible diagnostic emitted for the rule within the file where the rule
is implemented.
Visitor
callback, you have alsooverridden that callback method for the special
Validator
(
wdl-ast/src/validation.rs
) andLintVisitor
(
wdl-lint/src/visitor.rs
) visitors. These are required to ensure the newvisitor callback will execute.
wdl-gauntlet --refresh
to ensure that there are nounintended changes to the baseline configuration file (
Gauntlet.toml
).wdl-gauntlet --refresh --arena
to ensure that all of therules added/removed are now reflected in the baseline configuration file
(
Arena.toml
).