Overflowing string literal earlier in expression forces worse formatting

[jakemac53]

What input code did you provide to the formatter?

See the categories setter call at the bottom here.

base mixin DartAnalyzerSupport
    on ToolsSupport, LoggingSupport, RootsTrackingSupport, FileSystemSupport
    implements SdkSupport, ProcessManagerSupport {
  @visibleForTesting
  static final resolveWorkspaceSymbolTool = Tool(
    name: 'resolve_workspace_symbol',
    description:
        'Look up a symbol or symbols in all workspaces by name. Can be used '
        'to validate that a symbol exists or discover small spelling '
        'mistakes, since the search is fuzzy.',
    inputSchema: Schema.object(
      properties: {
        ParameterNames.query: Schema.string(
          description:
              'Queries are matched based on a case-insensitive partial name '
              'match, and do not support complex pattern matching, regexes, '
              'or scoped lookups.',
        ),
      },
      description:
          'Returns all close matches to the query, with their names '
          'and locations. Be sure to check the name of the responses to ensure '
          'it looks like the thing you were searching for.',
      required: [ParameterNames.query],
      additionalProperties: false,
    ),
    annotations: ToolAnnotations(title: 'Project search', readOnlyHint: true),
  )..categories = [FeatureCategory.dart, FeatureCategory.flutter, FeatureCategory.lsp];
}

What output did the formatter produce?

It won't change it (note that if you force a trailing comma for the categories setter it will reformat it to remove the trailing comma).

What output did you expect or want the formatter to produce?

I expected it to use trailing commas for the list passed to the categories setter to keep lines under 80 chars.

base mixin DartAnalyzerSupport
    on ToolsSupport, LoggingSupport, RootsTrackingSupport, FileSystemSupport
    implements SdkSupport, ProcessManagerSupport {
  @visibleForTesting
  static final resolveWorkspaceSymbolTool = Tool(
    name: 'resolve_workspace_symbol',
    description:
        'Look up a symbol or symbols in all workspaces by name. Can be used '
        'to validate that a symbol exists or discover small spelling '
        'mistakes, since the search is fuzzy.',
    inputSchema: Schema.object(
      properties: {
        ParameterNames.query: Schema.string(
          description:
              'Queries are matched based on a case-insensitive partial name '
              'match, and do not support complex pattern matching, regexes, '
              'or scoped lookups.',
        ),
      },
      description:
          'Returns all close matches to the query, with their names '
          'and locations. Be sure to check the name of the responses to ensure '
          'it looks like the thing you were searching for.',
      required: [ParameterNames.query],
      additionalProperties: false,
    ),
    annotations: ToolAnnotations(title: 'Project search', readOnlyHint: true),
  )..categories = [
    FeatureCategory.dart,
    FeatureCategory.flutter,
    FeatureCategory.lsp,
  ];
}

Anything else we should know?

This appears to be related to the string literals, if I shorten them, then it will reformat the list to use trailing commas, but it also indents the entire constructor call.

This would then cause the string literals to be >80 chars, which can't be fixed without manually editing them.

I would still prefer it do that though because that is something I can fix, and it isn't obvious otherwise that this is the actual cause, it just seems like a formatter bug.

[munificent]

Yeah, as you note, this is related to the string literals. The formatting style doesn't allow an unindented cascade with a split receiver like:

  static final resolveWorkspaceSymbolTool = Tool(
    ...
  )..categories = [
    ...
  ];

We could maybe support this, but I feel like it kind of buries the cascade in what could be (and is in your case) a very large expression and make it hard to notice. Instead, when a cascade setter's receiver and RHS both split, it forces the RHS to be more indented like:

  static final resolveWorkspaceSymbolTool =
      Tool(
          ...
        )
        ..categories = [
          ...
        ];

But that also pushes the body of the receiver over. In your case, that means the string literals in the descriptions no longer fit the page width. So it's presented with a piece of code where all ways that it could format it cause some lines to overflow. It takes the least-bad option in its eyes and picks the one where the list doesn't split and overflows instead.

I have some vague ideas on how it could handle overflowing lines like this more gracefully, but nothing concrete yet. Either way, what will help here is to reflow the strings so that they don't overflow when the Tool(...) argument list is indented, like:

base mixin DartAnalyzerSupport2
    on ToolsSupport, LoggingSupport, RootsTrackingSupport, FileSystemSupport
    implements SdkSupport, ProcessManagerSupport {
  @visibleForTesting
  static final resolveWorkspaceSymbolTool =
      Tool(
          name: 'resolve_workspace_symbol',
          description:
              'Look up a symbol or symbols in all workspaces by name. Can be '
              'used to validate that a symbol exists or discover small '
              'spelling mistakes, since the search is fuzzy.',
          inputSchema: Schema.object(
            properties: {
              ParameterNames.query: Schema.string(
                description:
                    'Queries are matched based on a case-insensitive partial '
                    'name match, and do not support complex pattern matching, '
                    'regexes, or scoped lookups.',
              ),
            },
            description:
                'Returns all close matches to the query, with their names '
                'and locations. Be sure to check the name of the responses to '
                'ensure it looks like the thing you were searching for.',
            required: [ParameterNames.query],
            additionalProperties: false,
          ),
          annotations: ToolAnnotations(
            title: 'Project search',
            readOnlyHint: true,
          ),
        )
        ..categories = [
          FeatureCategory.dart,
          FeatureCategory.flutter,
          FeatureCategory.lsp,
        ];
}

I'll leave this issue open as a note to see if I can come up with a more graceful way to degrade when it can't get lines to fit.

[jakemac53]

I have some vague ideas on how it could handle overflowing lines like this more gracefully, but nothing concrete yet. Either way, what will help here is to reflow the strings so that they don't overflow when the Tool(...) argument list is indented, like:

Yeah fwiw I did eventually figure out that manually editing the strings fixes the issue, but its very hard to discover this.

[munificent]

its very hard to discover this.

Oh, yeah. I have explained this issue to many people over the years. (Hmm, I should probably add this to the FAQ...)

[jakemac53]

Since I didn't capture it here already it's worth noting that the whole reason I even noticed this is that the 80 char line lint gets triggered by it. So the lint and the formatter are fighting each other and you can't make both happy unless you can root cause the issue to the long string literals.

Part of what makes it hard to realize the strings are the issue is that the strings are not the thing that ends up >80 chars, so the lint is very disconnected from the actual issue.

If I could wave a magic wand I would prefer the formatter to just make the strings overflow the 80 char limit here, because that would more directly point at the underlying problem and the fix would be immediately apparent (split the string literal).