Linter: improved docs for `sized_box_for_whitespace`

[kuhnroyal]

Describe the issue
The sized_box_for_whitespace lint is no longer working, in hindsight this may be the case since Flutter 2, not exactly sure, but I have changes in a bigger project where this lint should have kicked in for some 2-3 month.

To Reproduce

• flutter create
• setup flutter_lints (I have an own custom config, same problem there)
• add a Container() somewhere

Expected behavior
The analyzer should display the sized_box_for_whitespace warning for the Container().

Additional context
Reproducible on Flutter 2.2.2 and master

[pq]

Thanks for the report @kuhnroyal.

Do any of our current test cases produce lints for you?

https://github.com/dart-lang/linter/blob/master/test_data/rules/sized_box_for_whitespace.dart

[kuhnroyal]

Yes they do:

   info • Avoid unnecessary containers • lib/sized_box_for_whitespace.dart:10:10 • avoid_unnecessary_containers
   info • SizedBox for whitespace • lib/sized_box_for_whitespace.dart:16:10 • sized_box_for_whitespace
   info • SizedBox for whitespace • lib/sized_box_for_whitespace.dart:23:10 • sized_box_for_whitespace
   info • SizedBox for whitespace • lib/sized_box_for_whitespace.dart:30:10 • sized_box_for_whitespace
   info • SizedBox for whitespace • lib/sized_box_for_whitespace.dart:55:10 • sized_box_for_whitespace
   info • SizedBox for whitespace • lib/sized_box_for_whitespace.dart:62:10 • sized_box_for_whitespace

So probably my understanding of the lint is wrong and/or the lint has changed. Empty container with width/height previously was considered an error with this lint, now it seems to be accepted. But as soon as it has a Row() child it is considered bad.

Doesn't seem to make much sense to me. Would love some background info for this in the lint description.

[pq]

@jacob314 is a good person for semantics.

[kuhnroyal]

Looked at the actual lint class again and the description says a Container(width: x) is bad which makes sense but the lint doesn't catch it and the test says it is good.

I am pretty sure that the lint is bugged.

[pq]

Thanks for the feedback!

cc @jacob314 for his 2 cents.

[a14n]

AFAICT this works as intended.

SizedBox can only be used in a limited number of cases to be sure it will act the same as Container. If you look at the Container.build source code you will see that in some case a LimitedBox may be inserted in the widget tree. The lint takes that into account and doesn't trigger.

Basically the lint only triggers when there's:

• a child and (height or width)
• no child and both height and width

EDIT: perhaps it could be worth to update the description of the lint to mention how it works.

[kuhnroyal]

But the LimitedBox is then wrapped again with a ConstrainedBox.
So I have a ConstrainedBox -> LimitedBox -> ConstrainedBox.

When used as Container(width: 50), the outer ConstrainedBox has min/maxWidth = 50 which is effectively the same as SizedBox(width: 50). The child LimitedBox -> ConstrainedBox should not matter.

In any case, yes the docs need to be updated.

[a14n]

With the following snippet you will have a different output if you replace Container by SizedBox

import 'package:flutter/material.dart';

void main() => runApp(MyApp());

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: MyHomePage(),
    );
  }
}

class MyHomePage extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: ColoredBox(
        color: Color.fromARGB(255, 255, 0, 255),
        child: Container(height: 50),
      ),
    );
  }
}

[kuhnroyal]

I see, this used to be different and I assume the lint gave false positives.
So this issue should probably be considered a doc change request.

[pq]

Cool. @kuhnroyal: feel free to take a crack at improvements? Maybe @a14n or @jacob314 could help?