Formmatting and folding code issues

[slavizh]

Bicep version
Bicep CLI version 0.26.54 (5e20b29)

Describe the bug
There are two bugs/issues that I see with latest version. Most likely applicable for previous versions as well.
Issue 1

even that I have following formatting configuration:

"formatting": {
        "indentKind": "Space",
        "indentSize": 2,
        "insertFinalNewline": true,
        "newlineKind": "CRLF",
        "width": 160
    },

The width is not honored by if statements on resources. Those are always moved on the next line no matter what is the width defined. For me this is an issue as when possible I want to use the maximum width available and only multiline when that width threshold is reached. It should apply to any part of the code so if if statements do not exceed that threshold they should not be moved to new line.

Issue 2
When you have if statement on the resource and that if statement is moved to next line the whole indent is moved one space further. That causes the last closing bracket '}' to be indented and when you fold code it folds it to the next resource instead of folding it to the last bracket.

To Reproduce
test.bicep

param foo object

resource storageAccount 'Microsoft.Storage/storageAccounts@2023-01-01' =
  if (foo.deploy) {
    name: 'storageaccount1'
    kind: 'StorageV2'
    location: resourceGroup().location
    sku: {
      name: 'Standard_LRS'
    }
    properties: {
      accessTier: 'Hot'
    }
  }

resource roleAssignment 'Microsoft.Authorization/roleAssignments@2022-04-01' =
  if (foo.deploy) {
    name: 'roleAssignment1'
    properties: {
      principalId: ''
      roleDefinitionId: ''
    }
  }

Issue 1:

After document format:

Issue 2:

Additional context
Add any other context about the problem here.

[slavizh]

It seems also issue 1 applies for functions like map().

[shenglol]

Issue 1 stems from a deliberate choice made by the team, but I agree that wrapping if expressions can lead to indentation that is not aesthetically pleasing. Additionally, it appears that most customers prefer a longer width setting, diminishing the rationale for maintaining such wrapping. I'm going to modify the layout rule to exempt if expressions from being wrapped.

[shenglol]

Would you mind sharing an example for map()? I don't there is a special case added for it.

[dozer75]

@shenglol This issue started a few weeks ago for me as well, the bicep module ignores the formatting rules set in VS Codes and enforces a specific bicep code formatting... A format that I don't agree with at all. In addition to @slavizh examples this also true on other circumstances as well like e.g.:

var cosmosDbContributor = '00000000-0000-0000-0000-000000000002'
var cosmosRoleId = resourceId('Microsoft.DocumentDB/databaseAccounts/sqlRoleDefinitions', cosmosDbName, cosmosDbContributor)

Which will be formatted as

var cosmosRoleId = resourceId(
  'Microsoft.DocumentDB/databaseAccounts/sqlRoleDefinitions',
  cosmosDbName,
  cosmosDbContributor
)

A format I don't fancy at all.

Please revert the change, or at least configure it so that it honors VS Code formatting options. It would be nice that if you want to do something like this that you implement .editorconfig where WE can specify the rules for the formatting.

I have inactivated Format on Save for now to avoid having the bicep extension formatting my bicep files..

[slavizh]

map() example

param foo object

resource storageAccount 'Microsoft.Storage/storageAccounts@2023-01-01' =
  if (foo.deploy) {
    name: 'storageaccount1'
    kind: 'StorageV2'
    location: resourceGroup().location
    sku: {
      name: 'Standard_LRS'
    }
    properties: {
      accessTier: 'Hot'
    }
  }

resource roleAssignment 'Microsoft.Authorization/roleAssignments@2022-04-01' =
  if (foo.deploy) {
    name: 'roleAssignment1'
    properties: {
      principalId: ''
      roleDefinitionId: ''
    }
  }


resource appGw 'Microsoft.Network/applicationGateways@2023-09-01' = {
  name: 'tes1'
  properties: {
    backendHttpSettingsCollection: map(foo.array, array => {
      name: array.name
      id: array.id
    })
  }
}

on format with width 160 that is not reached:

[slavizh]

Basically we need strict formatting. Only when width is reached than format on new line. And if you have multiple statements do not put them all in new line just make them in a way that the new line honor the width.

[miqm]

I also don't like that formatter always moves arguments into new lines. Sometimes I by purpose split or join arguments into one line to add some comments. the new formatter breaks it all. it also treats a comment as an argument and moves it to a new line also.

It also moves trailing closing bracket of for-loops into 2 lines:
before

}]

now:

}
]

I'd like to keep it compact. At some point we decided that we allow objects and arrays to be defined in one line to not stretch bicep templates vertically. Now the formatter brings the problem back - templates are again unnecessarily expanding.

In my opinion, the new formatter is too aggressive.

[shenglol]

I also don't like that formatter always moves arguments into new lines.

The formatter respects the newline before the first array / object item. If you don't wrap the item then it will format it as single-line array / object, unless it exceeds the width limit. If long single-line array / object is preferred, you may simply increase the width value in bicepconfig.json.

Sometimes I by purpose split or join arguments into one line to add some comments.

Can you share an example? I think it only does so if joining them into one line results in parsing error. For instance, it is illegal to flatten

var myArray = [
   true // comment
   false ]

into

var myArray = [true // comment,  false ]

It also moves trailing closing bracket of for-loops into 2 lines

This decision is intentional to ensure uniform indentation levels, aligning with scenarios where an array of objects isn't produced by a for-loop expression.

var myArray  = [
  {
    foobar: 1
  }
  {
    foobar: 2
  }
]

var myArray = [
  for i in range(2): {
    foobar: i
  }
]

I understand that some might think the formatter is too strict in some cases. But it's tough to make a tool that everyone agrees with because people have different tastes. A formatter that is somewhat opinionated could be more useful, especially in team projects, because it makes things predictable and consistent and stops people from nagging about how the code looks during code review.

[shenglol]

@shenglol This issue started a few weeks ago for me as well, the bicep module ignores the formatting rules set in VS Codes and enforces a specific bicep code formatting... A format that I don't agree with at all. In addition to @slavizh examples this also true on other circumstances as well like e.g.:

var cosmosDbContributor = '00000000-0000-0000-0000-000000000002'
var cosmosRoleId = resourceId('Microsoft.DocumentDB/databaseAccounts/sqlRoleDefinitions', cosmosDbName, cosmosDbContributor)

Which will be formatted as

var cosmosRoleId = resourceId(
  'Microsoft.DocumentDB/databaseAccounts/sqlRoleDefinitions',
  cosmosDbName,
  cosmosDbContributor
)

A format I don't fancy at all.

Please revert the change, or at least configure it so that it honors VS Code formatting options. It would be nice that if you want to do something like this that you implement .editorconfig where WE can specify the rules for the formatting.

I have inactivated Format on Save for now to avoid having the bicep extension formatting my bicep files..

Just to clarify, there is a bicepconfig.json setting to control the behavior, but it's not through VS Code settings or .editorconfig, because we want to support Bicep CLI as well. You may increase formatting.width in bicepconfig.json to a big number like 240 to restore the old behavior.

[shenglol]

Basically we need strict formatting. Only when width is reached than format on new line. And if you have multiple statements do not put them all in new line just make them in a way that the new line honor the width.

Makes sense. The warpping of the map function is basically due to a rule where a multi-line child forces it's parent to be multi-line as well. For example,

var myArray = [
  [
    1
    2
  ], 3, 4]

will be formatted as

var myArray = [
  [
    1
    2
  ]
  3
  4
]

to improve readability, even if it doesn't exceed the width limit. However, the map function case is interesting because

backendHttpSettingsCollection: map(foo.array, array => {
  name: array.name
  id: array.id
})

is more compact and does look cleaner than

backendHttpSettingsCollection: map(
  foo.array,
  array => {
    name: array.name
    id: array.id
  
)

I'll add a heuristic to avoid breaking function argument list if the last element is an object.

Looks like Prettier has a similar heuristic :)

[dozer75]

Just to clarify, there is a bicepconfig.json setting to control the behavior, but it's not through VS Code settings or .editorconfig, because we want to support Bicep CLI as well. You may increase formatting.width in bicepconfig.json to a big number like 240 to restore the old behavior.

Ok, I'll take a look at the bicepconfig.json file, however are there any documentation of this file for formatting? The one in here doesn't mention the formatting.width file as far as I can see? The default configuration file has an empty formatting section as well here.

[o-l-a-v]

@dozer75

I found no doc either, but found that intellisense in VSCode will suggest available settings.

Would prefer a doc with all available options though.

[shenglol]

@mumian Do you mind helping add documentation for the new formatting settings in bicepconfig.json?

{
  "formatting": {
    "insertFinalNewline": boolean,       // Default value is true
    "newlineKind": "LF" | "CRLF" | "CR", // Default value is "LF"
    "indentKind": "Space" | "Tab",       // Default value is "Space"
    "indentSize": integer,               // Applicable when "indentKind" is set to "Space". Default value is 2.
    "width": integer,                    // The maximum number of characters that should appear on a line. Default is 120. The formatter will try to wrap the line if the width limit is reached.
  }
}

[dozer75]

I REALLY can't get this to work.

First I tried to create a bicepconfig.json file in my root folder for the specific project, but it still formats my document even though I set width to a large number (tried 140, 200, 500). I tried to restart VSCode as well to no avail (The line I test is 83 cols so it shouldn't be formatted in the first place).

Next, I tried to create a file from Visual Studio Code according to this documentation, but that doesn't create any file and just goes in a loop where I should specify the folder.

EDIT: This is the section I test on

module managedPrometheus 'managed-prometheus/main.bicep' = if (enablePrometheus) {
    name: 'managedPrometheus'
    params: {
      azureMonitorWorkspaceResourceId: azureMonitorWorkspaceResourceId
      clusterResourceId: aks.outputs.clusterId
      location: location
    }
  }

And it is formatted like

module managedPrometheus 'managed-prometheus/main.bicep' =
  if (enablePrometheus) {
    name: 'managedPrometheus'
    params: {
      azureMonitorWorkspaceResourceId: azureMonitorWorkspaceResourceId
      clusterResourceId: aks.outputs.clusterId
      location: location
    }
  }

[shenglol]

The if expression is something that we are going to fix for the next release (please see my response above here: #13676 (comment)). The width setting would work for the resourceId call you shared originally.

[dozer75]

The if expression is something that we are going to fix for the next release (please see my response above here: #13676 (comment)). The width setting would work for the resourceId call you shared originally.

Ok, I misunderstood then.. This is an overall problem that it messes with the code then and until it's fixed we'll have to disable format on save then so it doesn't mess with our code 👎

And by the way, I think you really need to revisit the whole formatting, not only on if as stated in your comment, this happens with other things as well.. Please leave our code formatting as we (the users) want it or give us the possibility to define our own rules... Refer to how it work in almost every other languages and tools, it is how it is for a reason... And implement .editorconfig as is more or less standard for these things...

[shenglol]

Refer to how it work in almost every other languages and tools, it is how it is for a reason...

I'd like to assure you that this was indeed what we've done.

JavaScript / TypeScript -> Prettier, Biome
Python -> Black, PEP8
Rust -> Rustfmt
Go -> Gofmt
C++ -> Clang-format

The formatters list above all implemented the same or similar pretty-printing algorithm to enforce opinionated formatting rules with length / width limits. The offical C# formatter (dotnet format) does not have the capability yet, but the issue Prettier-style line breaking has over 100 upvotes now, and there is a community formatter CSharpier that implements it today.

I understand having strong opinions about the "right" code style, but I think the general consensus of many people is that it doesn't matter what the specifics are, as long as they're being applied consistently. As pointed out by Prettier's Option Philosophy, the idea of having consistent formatting rules is to stop all the ongoing debates over styles like this.

That being said, one potential solution to accommodate varying preferences could be implementing a feature like #format disable. This directive would allow teams to opt out of automated formatting on specific code blocks where manual formatting is preferred. Would this approach meet your needs?

Discussing the .editorconfig, it seems there might be some confusion about how it works. Essentially, .editorconfig sets basic coding styles for text editor plugins (e.g., for VS Code, you would have to install the EditorConfig extension). Its rules come into effect when the text editor applies them after the code has been formatted by the language server. It's not designed as a code formatting standard for language servers. AFAIK, C# is the only language that uses it to configure custom coding styles and linter rules. While it's technically feasible for the Bicep CLI and Bicep language server to do the same, but since we already utilize bicepconfig.json to define linter rules, to maintain consistency, there are no plans to integrate .editorconfig support.

[dozer75]

I will not continue to try reasoning to have a more flexible solution now it isn't worth the time. If you just fix so we can use width in all scenarios to avoid having the formatter to automatic line breaks so I can get back the old functionality and I will be ok with that.

[dstarkowski]

@shenglol Is there a plan to change the formatting of for loops as well?
The if block was changed to stay in the same line in v0.27.1, but for is still moved to the new line.
It was just annoying before - now it's annoying and inconsistent :D