Skip to content
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: add lint warning for newline_eof #18

Merged
merged 1 commit into from
May 2, 2024

Conversation

simojoe
Copy link
Contributor

@simojoe simojoe commented Apr 17, 2024

This pull request adds a new rule to wdl.

  • Rule Name: newline_eof
  • Rule Kind: Lint warning
  • Rule Code: v1::W007
  • Packages: wdl-grammar

This rule is currently written as to avoid missing newline at the end of WDL files.

My interpretation of the rule is that a WDL file should not end with an empty line (double newline symbols), but the rule description is not explicit about that (or the opposite situation).

The implementation currently checks two things :

  • there is at least one newline at the end of the file
  • there is at most one newline at the end of file

Let me know if this implementation is not suitable

Before submitting this PR, please make sure:

  • You have added a few sentences describing the PR here.
  • You have added yourself or the appropriate individual as the assignee.
  • You have added at least one relevant code reviewer to the PR.
  • Your code builds clean without any errors or warnings.
  • You have added an entry to the relevant CHANGELOG.md (see
    ["keep a changelog"] for more information).
  • Your commit messages follow the [conventional commit] style.
  • Your changes are squashed into a single commit (unless there is a really
    good, articulated reason as to why there shouldn be more than one).

Rule specific checks:

  • You have added the rule as an entry within the the package-specific rule
    tables (wdl-ast/src/v1.rs for AST-based rules and
    wdl-grammar/src/v1.rs for parse tree-based rules).
  • You have added the rule as an entry within the the global rule
    table at RULES.md.
  • You have added the rule to the appropriate fn rules().
    • Validation rules added to wdl-ast should be added to fn rules() within
      wdl-ast/src/v1/validation.rs.
    • Lint rules added to wdl-ast should be added to fn rules() within wdl-ast/src/v1/lint.rs.
    • Validation rules added to wdl-grammar should be added to fn rules() within
      wdl-grammar/src/v1/validation.rs.
    • Lint rules added to wdl-grammar should be added to fn rules() within
      wdl-grammar/src/v1/lint.rs.
  • You have added a test that covers every possible setting for the rule
    within the file where the rule is implemented.

wdl-core/src/concern/lint/group.rs Show resolved Hide resolved
wdl-grammar/src/v1.rs Outdated Show resolved Hide resolved
wdl-grammar/src/v1/lint/newline_eof.rs Outdated Show resolved Hide resolved
wdl-grammar/src/v1/lint/newline_eof.rs Outdated Show resolved Hide resolved
@claymcleod claymcleod added the enhancement New feature or request label Apr 18, 2024
@simojoe
Copy link
Contributor Author

simojoe commented Apr 22, 2024

Updated the PR.

The only thing that I am unsure about is for Gauntlet.

I am running cargo run --bin wdl-gauntlet -- --save-config to generate the Gauntlet.toml file, but I am getting stuck behind the rate limit. Is there a way that I can use my personal GitHub account to fill the repository cache for wdl-gauntlet ?

@claymcleod claymcleod force-pushed the feat/newline_eof branch 3 times, most recently from f20c805 to 5dd1555 Compare May 2, 2024 14:42
This commit squashes the following commits:

* docs: updating Changelog
* feat: newline_eof logic and doc update
* feat: updating newline_eof tests
* feat: updating gauntlet.toml for newline_eof
@claymcleod claymcleod merged commit 35731ec into stjude-rust-labs:main May 2, 2024
6 checks passed
@peterhuene
Copy link
Collaborator

@simojoe FYI, you may set the GITHUB_TOKEN environment variable to a personal GitHub access token, which should help getting pass the rate limiting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants