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

New rule: DisallowedDeclarationName #215

Open
a-frantz opened this issue Oct 9, 2024 · 5 comments · May be fixed by #343
Open

New rule: DisallowedDeclarationName #215

a-frantz opened this issue Oct 9, 2024 · 5 comments · May be fixed by #343
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@a-frantz
Copy link
Member

a-frantz commented Oct 9, 2024

Credit to @adthrasher for the idea proposed in a comment on #133

We have DisallowedInputName and DisallowedOutputName. Those should remain unchanged, but there should be a new rule that checks all Decls (IO and private) for "type prefixes/suffixes". e.g. File gtfffile would be bad and so would Int my_int

@a-frantz a-frantz added enhancement New feature or request good first issue Good for newcomers labels Oct 9, 2024
@adthrasher
Copy link
Member

One consideration (via Mike Rusch), what about something like bcftools annotate (https://samtools.github.io/bcftools/bcftools.html#annotate). It has parameters that can be specified as a string or as a file. So the file suffix might make sense in that case to distinguish.

-r, --regions chr|chr:pos|chr:from-to|chr:from-[,…​]
see [Common Options](https://samtools.github.io/bcftools/bcftools.html#common_options)

-R, --regions-file file
see [Common Options](https://samtools.github.io/bcftools/bcftools.html#common_options)

@Gyan-max
Copy link

Gyan-max commented Mar 3, 2025

Hi @a-frantz and @adthrasher
I'd love to work on this issue.
Here is my plan:

  1. Review the existing rules (DisallowedInputName, DisallowedOutputName) to understand how they work.
  2. Add a new DisallowedDeclarationName rule to catch bad names in declarations.
  3. Write tests for valid/invalid cases.
  4. update the docs to explain the new rule.

I would like to confirm if this plan is good. Please let me know if there is any additional guidelines i should follow.

@claymcleod
Copy link
Member

This looks good. Let us know if you have any questions!

@Gyan-max
Copy link

Gyan-max commented Mar 8, 2025

Hey @claymcleod , I'm trying to test my new DisallowedDeclarationName rule but running into issues with the shellcheck tests failing. These failures seem unrelated to my code - they're showing different shellcheck diagnostics than what's expected in the test files. Is there a way to run just my specific test without the shellcheck tests interfering? Or should I ignore these errors for now?

@Gyan-max
Copy link

Gyan-max commented Mar 8, 2025

Although I'm submitting a PR for this, please review it also and suggest to me if any changes are needed.

@Gyan-max Gyan-max linked a pull request Mar 8, 2025 that will close this issue
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants