Feature/remove sheriff.config from lint programatically

[tomwhite007]

Per your comment, 2nd bullet point: #161 (comment)

This PR adds the ability for rules to have a filename inclusions or exclusions list set inside the createRule factory and filters the 'sheriff.config.ts' file from all the existing rules. This allows a future set of rules that can be filename/type-specific.

Of my two PRs this is the most testable, but both are written with consideration to solving #165.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[tomwhite007]

With hindsight, if you like this PR, I'd be happy to write more unit tests to cover all possible include/exclude configurations.

[tomwhite007]

Closing this one as alternative PR, #167 was accepted instead of this one.

I'll now start looking into a solution for #165

Thanks @rainerhahnekamp

[rainerhahnekamp]

@tomwhite007, would it be possible to reopen that one? While I see #167 as a quick fix, the most optimal solution would involve implementing a separate initialization process. I understand this might require a bit more planning, but with this PR, we’d already have a solid working draft to build on.

What are your thoughts?

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[tomwhite007]

Hi @rainerhahnekamp

Yes, happy to reopen. I'll come back with my thoughts soon.

[rainerhahnekamp]

Great, maybe we can do some brainstorming here on top of your work.

[tomwhite007]

Hi Rainer

OK, I'd like to understand your perspective a little more, but I'll start making some suggestions.

My objective is to provide feedback on any bad configuration in sheriff.config file, per #165 . My immediate thought to solve this would be via an entirely new eslint rule. That's why the #167 PR you have merged would act as a starting point because the existing set of rules now filters out the sheriff.config file, and a new one would be configured to only act on sheriff.config alone.

However, I get the feeling you have a more far-reaching goal. Is it that the feedback for bad configuration in the sheriff.config file should come from the Sheriff command line (sheriff verify) as well as eslint? If that's the case, then I want to understand what you would like to happen around command line parameters.

At the moment, sheriff verify expects the path to main.ts as the starting point. But to run on the sheriff.config file, we could either pass the sheriff.config file name or —config. Then this PR would be useful because we can configure a new createRule() for sheriff config validation where sheriff.config.ts is on the include list rather than the exclude.

You mentioned a separate initialization process. Do either options above fit the bill? Or, how / where do you expect the separate initialization should take place?

I’m not precious about the above suggestions. You know your codebase better so point me at a different file, and I’ll start digging if you want me to.

[rainerhahnekamp]

So I see two issues that need addressing:

1. Config-Specific Errors During Linting:

   • When we lint a file and an error arises due to a problem in the sheriff.config.ts file, we need to surface that somehow. My initial thought was to show these config-specific errors only when the linter runs directly against sheriff.config.ts, but this creates a problem: if the linter is run against another file, everything may look fine, even though the configuration is invalid.
   • Therefore, I think config-specific errors per file need to stay, as they provide necessary context.

2. Nx-Related Behavior:

   • There’s an issue when running Sheriff in an Nx workspace. I think this can be resolved by ensuring that the default Sheriff process does not run against sheriff.config.ts.

---

Proposed Approach

• We could introduce special linter errors for subtle issues in sheriff.config.ts. These wouldn’t block Sheriff entirely but would flag things like:
  • Providing a path that doesn’t exist.
  • Using a deprecated property like tagging.
• These subtle issues should appear only when linting sheriff.config.ts.

At the same time, config-related errors that already surface when Sheriff is run against TypeScript files should also show up when linting sheriff.config.ts.

---

From a high-level perspective, I don’t think linting the config file needs to go through the same initialization process we use for TypeScript files. However, we’ll need to distinguish between:

1. Blocking User Errors: Issues that make Sheriff fail entirely (e.g., invalid paths).
2. Non-Blocking Config Issues: Subtle misconfigurations that should be flagged but don’t prevent Sheriff from running.

At the moment, both are of type UserError. There's a file which lists all of them in the source code.

[tomwhite007]

I had to re-read your comment a few times to absorb. Regarding your two issues, or requirements: 1. Agreed - sounds like a good idea. We need a lint rule specifically for sheriff.config.ts, and then we need part of that same process to run and throw UserError when the other Sheriff rules run to block them when there is misconfiguration... I propose two stages, a) I build and then iterate from a new eslint rule for the sheriff.config.ts (including Blocking and Non-Blocking checks) - with a view to - b) sharing the process to throw the UserError for Blocking misconfiguration in existing rules. 2. The sheriff.config.ts is already filtered out using the eslint config in my last PR. I propose that we keep it this way and my PR for point 1 above will clarify why that's easier, given your requirements. The new eslint rule will have its own eslint config that only includes the /sheriff.config.ts filepath. How does that sound?

[rainerhahnekamp]

I had to re-read your comment a few times to absorb.

Sorry for that. Sometimes I express my thoughts a little bit too complicated.

I propose two stages, a) I build and then iterate from a new eslint rule for the sheriff.config.ts

Sounds great to me Tom!