Allow the suppression of BCP334 and BCP335 inside bicep's configuration file

[Gabriel123N]

Is your feature request related to a problem? Please describe.

Since the latest version of Bicep (0.16 I think?), two new warnings have been added to the linter:

• BCP 334 giving a warning if a parameter could be shorther than the minimum length
• BCP 335 doing the same for the maximum length

While it makes sense to have such warnings, one of the issues is that if your bicep templates are reliant on functions such as resourcegroup().name to generate resource names, inputs or outputs, a warning will be outputed everytime that function is called anywhere in your module.

Assuming you respect a naming convention such as the one from the Azure periodic table, you will often be calling code similar to this one param resource_,name string = replace(resourcegroup().name, '-rg-', '-xxx-') which leads for many warnings for these calls if you add min/maxLength.

Another issue related to the resourcegroup().name function and resource outputs, is that the maxLength and minLength do not take into account the limitations on said resources.

With the following code:

@minLength(1)
@maxLength(90)
param log_analytics_scope string = resourcegroup().name

A warning will be outputted both for the BCP334 and 335 due to the default value. However, resourcegroup().name is already constrained between 1 and 90 characters.

For outputs this following code will also trigger the two warnings:

//main.bicep

module apim 'path1' = {
...
}

module api 'path2' = {
...
params: {
...
apim_name: apim.outputs.apim_name //BCP334 and BCP335 triggered here
...
}

// apim module

resource apim 'apim service namespace' = { ... }

output apim_name string = apim.name

// api module

@minLength(1)
@maxLength(50)
param apim_name string

Again, BCP334 and BCP335 are triggered while it is technically impossible for the input to break the parameter constraints as an Api Management service name is between 1 and 50 characters

Finally, even by explicitly putting constraints on outputs, if it is passed as parameter to another module, BCP334/335 will still be triggered.

Using my previous apim example, if I replace the output to this line:

output apim_name string = take(apim.name, 50)

BCP335 is still triggered even though the take function should limit the output to 50 characters

Result: A basic deployment deploying less than 10 resources and a few service bus queues in our environment is triggering over 100 warnings making it harder to find real warnings or errors if the deployment fails

Describe the solution you'd like It should be possible to disable the two warnings mentioned directly from the .bicepconfig file as it is extremely easy to fire them even when the inputs/outputs are controlled.

I know it is possible to disable them with the #disable-next-line statement but it is not a solution to have to do this hundreds of times on many files while it should be something that could be done in 5 lines in a central place.

[StephenWeatherford]

Note that there is a plan to add #disable-file, which would help somewhat (#5568).

That said, I believe we should prioritize fixing any linter/compiler warning to not fire in common cases like these rather than encouraging wholesale disabling (compiler or linter).

Also, should compiler warnings/errors participate in bicepconfig.json (including setting levels and obeying warningsAsErrors if we add that #3851)?

[alex-frankel]

I think this got resolved with #10398 and will be fixed in the next release on 5/1. Can you take a look @Gabriel123N and confirm if it is the same issue?

[jeskew]

@alex-frankel I think this issue is different from #10398. In this case, the user knows that an expression (replace(resourcegroup().name, '-rg-', '-xxx-')) will satisfy a parameter's constraints, but the compiler can't really predict what that expression will do at runtime and so is issuing warnings that are technically correct but not helpful.

I wonder if BCP334 and BCP335 should be downgraded to an Informational level suppressed when a value has no discernible minimum or maximum length. Issuing a warning in the absence of information seems more pedantic than issuing a warning when the compiler knows the minimum length of a value and can see that it's lower than the required minimum length of an assignment target. (Edited to add: Info diagnostics still show up in build logs and in VS code, so we should probably suppress the diagnostic entirely in the absence of min/max length info.)

[alex-frankel]

I agree that we should suppress these if they are not resolvable.

[StephenWeatherford]

This also seems to beg the question of what warnings should be compiler warnings and what should be linter warnings. This one feels more like a linter rule, but is easiest implemented by the compiler.

[Gabriel123N]

I think this got resolved with #10398 and will be fixed in the next release on 5/1. Can you take a look @Gabriel123N and confirm if it is the same issue?

It does offer a way to prevent warnings when using outputs from modules. However, it still wouldn't prevent it when passing a property of a created resource with known constraints directly.

Example:

resource apim "apim/service namespace"  = {
   name: 'apim-name' // Api management service name constraints are static (1<= x <= 50)
   ...
}

module apim_kv_assignment = './apim-kv-role-assignment.bicep' = {
...
params: {
   apim_name: apim.name //BCP334 and BCP335 if minLength = 1 and maxLength = 50 for param api_name
}
}

I believe Jeskew resumed be the core problem well. If the compiler cannot predict how long/short an output will be, in my opinion, the log/linter level produced by that behaviour should be customizable by the end user.

[jeskew]

It does offer a way to prevent warnings when using outputs from modules. However, it still wouldn't prevent it when passing a property of a created resource with known constraints directly.

This feature is planned but is unlikely to be ready prior to the 0.18 release. But even with that feature in place, not every resource property has its constraints accurately reflected in the swagger service models used by Bicep for type checking.

the log/linter level produced by that behaviour should be customizable by the end user.

We have a few options here:

1. Get rid of BCP334 and BCP335. When the compiler checks whether a value assignment will succeed, the three possible answers are "yes," "no," and "maybe." Bicep generally emits an error when the answer is "no," but there's some inconsistency around how "maybe" cases are handled:

   // sometimes, Bicep says nothing and lets the runtime sort it out
   output foo string = any(true)
   
   // sometimes, Bicep emits an error
   param cereal 'snap' | 'crackle' | 'pop'
   param myParam 'fizz' | 'buzz' | 'pop' = cereal // <-- this will succeed at runtime if cereal == 'pop', but the compiler treats this as a compilation-blocking error
   
   // with BCP334/BCP335, sometimes Bicep will emit a warning
   @minLength(1)
   param short string
   @minLength(2)
   param notAsShort string = short

2. Allow compiler diagnostics to be configured via bicepconfig (just like linter diagnostics).

3. Disable BCP334/BCP335 when the assigned value has no length metadata (i.e., treat any length metadata as an opt-in for refinement checking):

   // this would eliminate BCP335 for cases where the compiler throw up its hands
   @maxLength(50)
   param deployTimeDecidableDefault = replace(resourceGroup().name, '-rg-', '-xxx-')
   
   // but would keep it for cases that look more like an authoring error
   @maxLength(50)
   param boundedRgName string = take(resourceGroup().name, 60)

[jeskew]

Based on a team discussion, we decided to go with option 3 (only emit BCP334 when a string/array has a specified min length AND it's less than the min length of the target & only emit BCP335 when a string/array has a specified max length AND it's greater than the max length of the target), with a note to revisit option 2 (control via bicepconfig.json) for a later release.