Refine diagnosticFilters

[TwitchBronBron]

DiagnosticFilters is a bit weird.
if you give it numbers, those apply to only bsc diagnostics.
If you give it strings, those apply to a relative srcPath to the actual source files.
you have to use {codes: []} to target string-based diagnostics.

This is a breaking change. I think we should modify the diagnosticFilters accordingly:

• top-level integers and strings should be diagnostic codes which are used to filter the entire project. Internally, they are mapped to { codes: []}
• we should support specifying the string style of bsc diagnostics (i.e. "bs1234") in addition to 1234...but maybe we should disable the pure integer version entirely since that's not how they're printed...ever
• if you want to ignore files, you can specify a files section in the object, which follows the same rules as the outer files array.

Here's an example bsconfig with these options applied:

"diagnosticFilters": [
    1001,
    "bs1002",
    "lint1003",
    {
        "codes": [
            1004,
            "bs1005",
            "lint1006"
        ]
    },
    {
        "files": [
            // a srcPath that is also a destPath
            "source/common.brs",
            {
                "src": "../themes/theme1/**/*"
            }
        ]
    },
    {
        "files": "**/roku_modules/**/*",
        //exclude certain codes from certain files
        "codes": [
            1200,
            "bs1201"
        ]
    }
],

[josephjunker]

Adding the string diagnostics code would be a big win. Our developers encounter friction there pretty regularly.

Question, why would the full files object be permitted here, rather than files solely being a source path? I'm imagining a bsconfig like this:

{
    files: [
        {
            src: "foo"
            dest: "bar"
        }
    ],
    diagnosticFilters: {
        files: [
            {
                src: "foo"
                dest: "baz"
             }
        ]
    }
}

What would the behavior be, since src matches but dest does not? It feels to me like the meaning of dest in a diagnostic filters would be confusing. And if you can't use dest, then what would the point of using src be?

Also when you say "follows the same rules as the outer files array", does this mean you could prefix a file path with ! to "un-ignore" it? I could see that being very useful when progressively enabling checks on an existing codebase, but it also sounds like it would complicate the implementation.

[TwitchBronBron]

What would the behavior be, since src matches but dest does not? It feels to me like the meaning of dest in a diagnostic filters would be confusing. And if you can't use dest, then what would the point of using src be?

Very good point.

My goal is to support excluding diagnostics based on their src paths OR their dest paths, whichever makes more sense for your project setup. The diagnosticFilters design in 0.* is not obvious whether the current path strings in diagnosticFilters are src or dest. (they're src btw).

Do you have any suggestions on how to best support that goal of specifying src paths OR dest paths with_ this redesign?

[josephjunker]

Would it make sense to allow for dest or src to be specified, but not both in the same object?

The way I see it is, if they're both included in the same object then you have two options for what that object can match: the union of the files matched by each glob, or the intersection of the files matched by each glob. Neither one strikes me as particularly intuitive. If you can only specify one at a time though you don't have to make a decision on which "combining" logic to use.

[josephjunker]

Put another way-- if you wanted "union" behavior then you can already have that, by just putting two separate entries in the array. If you wanted "intersection" behavior... I don't think you want intersection behavior 😅. It feels like it would require a lot of reasoning about the project structure, and I'm not clear on what the use-case would be.

[TwitchBronBron]

Would it make sense to allow for dest or src to be specified, but not both in the same object?

I think this suggestion makes the most sense. Would this be clear enough as a consumer (assuming we have some validation there to warn about specifying both)?

I mean, worst case, we could just have union behavior, where we add the list of src paths and dest paths into a big list rather than intersection. SO it's like a way to specify paths just in two different buckets.

[josephjunker]

From my perspective, allowing dest XOR src would be clear, and would require less explanation than the union behavior (though yes it would definitely be helpful to have good validation messages like you said.) I can't claim to speak for every bsconfig.json author though so this is just my 2 cents.

[TwitchBronBron]

Yeah, I agree. Let's just go with { src} OR {dest} but not {src;dest}