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

dclint disable rule-name doesn't appear to be disabling rule-name #69

Closed
wesley-dean-flexion opened this issue Dec 11, 2024 · 4 comments · Fixed by #88
Closed

dclint disable rule-name doesn't appear to be disabling rule-name #69

wesley-dean-flexion opened this issue Dec 11, 2024 · 4 comments · Fixed by #88
Assignees
Labels
bug Something isn't working released

Comments

@wesley-dean-flexion
Copy link

Version

2.1.0

YAML file where the error occurs

---
# dclint disable require-project-name-field
# name: "does_not_matter_commented_out"

Command used to run

dclint ./docker-compose.yml

Optional configuration (if different from the default)

n/a

Additional context

node: 20.18.1

Current behavior (console output)

# dclint --debug ./docker-compose.yml
[DEBUG] [CLI] Debug mode is ON
[DEBUG] [CLI] Arguments: {
  _: [],
  debug: true,
  recursive: false,
  r: false,
  fix: false,
  'fix-dry-run': false,
  fixDryRun: false,
  formatter: 'stylish',
  f: 'stylish',
  quiet: false,
  q: false,
  color: true,
  exclude: [],
  e: [],
  'max-warnings': -1,
  maxWarnings: -1,
  '$0': 'dclint',
  files: [ './docker-compose.yml' ]
}
[DEBUG] [CONFIG] Configuration file not found. Using default
[DEBUG] [CLI] Final config: { rules: {}, quiet: false, debug: true, exclude: [] }
[DEBUG] [UTIL] Looking for compose files in ./docker-compose.yml
[DEBUG] [UTIL] Paths to exclude: node_modules,.git,.idea,.tsimp
[DEBUG] [UTIL] Found compose files in ./docker-compose.yml: ./docker-compose.yml
[DEBUG] [LINTER] Compose files for linting: ./docker-compose.yml
[DEBUG] [LINTER] Linting file: ./docker-compose.yml
[DEBUG] [LINTER] Linting result: [{"filePath":"./docker-compose.yml","messages":[{"rule":"require-project-name-field","type":"warning","category":"best-practice","severity":"minor","message":"The \"name\" field should be present.","line":1,"column":1,"meta":{"description":"Ensure that the \"name\" field is present in the Docker Compose file.","url":"https://github.com/zavoloklom/docker-compose-linter/blob/main/docs/rules/require-project-name-field-rule.md"},"fixable":false}],"errorCount":0,"warningCount":1,"fixableErrorCount":0,"fixableWarningCount":0}]
[DEBUG] [UTIL] Using built-in formatter: stylish

/path/to/docker-compose.yml
   1:1     warning  The "name" field should be present.  require-project-name-field

✖ 1 problems (0 errors, 1 warnings)

Expected behavior


(i.e., no output because no errors or warnings were found)

Praise

I stumbled upon dclint and I was immediately impressed by the frequency of updates, the list of PRs being clean the inclusion of linters in .github/workflows, the quality of the documentation, the use of common patterns to enable/disable rules, produce output, and so on. This looks like a model project and I don't know how I haven't seen it before.

I've contributed a few things to https://github.com/oxsecurity/megalinter and my first thought was to investigate including it in the list of linters MegaLinter supports. See oxsecurity/megalinter#4381

This isn't related to the bug report -- I just wanted to express my appreciation.

Thank you for all of your efforts and hard work.

@wesley-dean-flexion wesley-dean-flexion added the bug Something isn't working label Dec 11, 2024
@zavoloklom zavoloklom self-assigned this Dec 14, 2024
@zavoloklom
Copy link
Owner

@wesley-dean-flexion Hi!

First of all, thank you so much for the feedback, I really appreciate it and it means a lot.

Regarding the bug:

The dclint disable instruction only works on the first line, so it should look like this:

# dclint disable require-project-name-field
---
# name: "does_not_matter_commented_out"

I personally rarely use delimiters, so I’d be glad to hear your advice — should I configure the search to ignore --- and treat the comment after it as the first line, or is the way I described above also convenient?

@wesley-dean-flexion
Copy link
Author

@zavoloklom Thank you for pointing that out. As I review the documentation, I do see that it mentions that the dclint disable needs to be at the top (original emphasis) of the file. Clearly this is not a bug and, rather, a misunderstanding on my part. I apologize for my lack of attention to detail.

I affirmatively use delimiters for all of my YAML because, if for no other reason, so that yamllint doesn't produce a finding:

https://yamllint.readthedocs.io/en/stable/rules.html#module-yamllint.rules.document_start

I went to the YAML spec to see what it said:

https://yaml.org/spec/1.2.2/#22-structures

YAML uses three dashes (“---”) to separate directives from document content. This also serves to signal the start of a document if no directives are present. Three dots ( “...”) indicate the end of a document without starting a new one, for use in communication channels.

(Today I Learned that ... can be used to send a document without starting a new one)

So, to your question, I would be inclined to not include the document start notation (---) when considering if the directive appears on the first line. My interpretation of the spec -- right, wrong, or otherwise -- is that the YAML content doesn't actually begin until after the first document starts, so # dclint disable isn't a YAML comment, but rather a string that happens to start with #.

I think that my interpretation is fairly pedantic and for most, a distinction without a difference.

Personally -- and this is just a thought -- I would appreciate it if the document start and blank lines were ignored at the top of a file. If blank lines were not ignored, the following wouldn't be correct:

---

# dclint disable [...]

services:
[...]

This may be confusing to some (including me).

All of that said, it if your project and I respect your decisions about what to change or not change -- it's not my place to make any demands of you or your time.

I want to reiterate, however, that you're doing an amazing job and I appreciate your time, your work, and your decision to contribute the fruits of your labor to the rest of the world.

zavoloklom added a commit that referenced this issue Dec 18, 2024
zavoloklom added a commit that referenced this issue Dec 18, 2024
zavoloklom added a commit that referenced this issue Dec 18, 2024
zavoloklom added a commit that referenced this issue Dec 18, 2024
@zavoloklom
Copy link
Owner

@wesley-dean-flexion Thank you for sharing your detailed feedback and insights - it’s incredibly valuable to me. I appreciate the time and effort you’ve taken to explore the nuances of YAML and share your perspective.

I’ve addressed this issue in version 2.2.1 of the project. I hope this resolves any confusion and improves the user experience.

Once again, thank you for your comments and support. It’s feedback like yours that helps refine and improve the tool for everyone. If you notice anything else or have further suggestions, don’t hesitate to reach out!

@zavoloklom
Copy link
Owner

🎉 This issue has been resolved in version 2.2.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
2 participants