dart_style icon indicating copy to clipboard operation
dart_style copied to clipboard

Chained methods with closures that have a body leads to incorrect indentation

Open rrousselGit opened this issue 4 years ago • 6 comments

Similar to #137, but for chained methods

Consider:

final commonParameters = constructrorsNeedsGeneration
    .first.parameters.allParameters
    .where((parameter) {
  return constructrorsNeedsGeneration.every((constructor) {
    return constructor.parameters.allParameters.any((p) {
      return p.name == parameter.name && p.type == parameter.type;
    });
  });
}).toList();

The indentation of the body of the .where clause is unexpected (2 spaces instead of 6)

I would expect:

final commonParameters = constructrorsNeedsGeneration
    .first.parameters.allParameters
    .where((parameter) {
      return constructrorsNeedsGeneration.every((constructor) {
        return constructor.parameters.allParameters.any((p) {
          return p.name == parameter.name && p.type == parameter.type;
        });
      });
    }).toList();

Note that this only happens with closures using {}. Closures using => don't have this issue and are formatted this way:

final commonParameters = constructrorsNeedsGeneration
    .first.parameters.allParameters
    .where((parameter) => constructrorsNeedsGeneration.every(
        (constructor) => constructor.parameters.allParameters.any(
            (p) => p.name == parameter.name && p.type == parameter.type)))
    .toList();

rrousselGit avatar Mar 12 '20 15:03 rrousselGit

It's a problem with formatting closures with a code block in general.

// ACTUAL

import 'dart:io';

void main() {
  final f = stdin.readLineSync,
      p = int.parse,
      n = p(f()),
      ranges = List.generate(n, (_) {
    final t = f().split(' '), a = p(t[0]), b = a + p(t[1]);
    return [a, b];
  }),
      dp = List<int>(n);
}



// EXPECTED

import 'dart:io';

void main() {
  final f = stdin.readLineSync,
      p = int.parse,
      n = p(f()),
      ranges = List.generate(n, (_) {
        final t = f().split(' '), a = p(t[0]), b = p(t[1]);
        return [a, b];
      }),
      dp = List<int>(n);
}

jolleekin avatar Mar 19 '20 07:03 jolleekin

@rrousselGit, your example is "deliberate" in that I'm not surprised it's what the formatter does, but I agree it doesn't look great. The challenge is that there are many cases where not indenting lambdas is clearly much better. The test API is the canonical example. It would be a drag if every lambda passed to test() and group() had its entire body indented +4. So the formatter has some heuristics to decide when to give lambdas "block-like" indentation versus normal expression indentation. Those heuristics work well most of the time, but definitely have some failure cases like your example here.

This is one of the more complex parts of the formatter and it interacts with the line splitting algorithm in subtle ways, so it's hard to improve cases like yours without either making more other examples worse or causing performance problems. It's an area I'd love to revisit and try to totally overhaul, but I haven't had the time.

@jolleekin, your example is different. This one is just about not increasing the indentation level in multiple-variable declarations. It's very rare that users declare multiple variables with a single statement in Dart and even rarer for one of the initializers to have a complex body like your example here, so I think I just never ran into this case and decided how to handle it.

A simple workaround, which is more idiomatic in Dart anyway, is to change your code to:

void main() {
  final f = stdin.readLineSync;
  final p = int.parse;
  final n = p(f());
  final ranges = List.generate(n, (_) {
    final t = f().split(' '), a = p(t[0]), b = p(t[1]);
    return [a, b];
  });
  final dp = List<int>(n);
}

munificent avatar Mar 20 '20 17:03 munificent

@munificent Of course I know it will work if I just use single variable declarations. The reason I use the multi-variable declarations is because it's much shorter and faster to type, which I find very convenient in solving programming challenges.

jolleekin avatar Mar 23 '20 09:03 jolleekin

OK, I've landed a change that fixes your case. It will go out in the next release of dartfmt.

munificent avatar Mar 24 '20 00:03 munificent

Nice. Does this also close #755?

passsy avatar Mar 24 '20 16:03 passsy

No, it only addresses leading indentation in multiple variable declarations. Indentation in method chains is a much harder problem.

munificent avatar Mar 24 '20 16:03 munificent