dart-lang / language

Design of the Dart language
Other
2.61k stars 200 forks source link

Inheritance and shadowing between parent files and part files. #3862

Closed lrhn closed 1 week ago

lrhn commented 1 month ago

I specified "part files with imports" (https://github.com/dart-lang/language/pull/3800) to generalize over "augmentation libraries" and part files, so that part files can have import (etc.).

@jakemac53 had some suggestions on abilities of part files to shadow names inherited from the parent file, which differ from the current specification. The options currently on the table can be summarized as:

The four combinations would have the following top-level scopes:

So, which version do we want, or are there more options?

@dart-lang/language-team

jakemac53 commented 1 month ago

I think it is useful to consider the scopes of the parent files as well as the parts, to develop a consistent and principled approach here.

In particular, there are actually three declaration scopes to consider - the current files declaration scope, the parent (part of) declarations scope, and the declaration scopes of all the parts (children) of the current file.

I am quite opposed to the idea of any shadowing of anything from any of these declaration scopes. That seems highly unexpected - and we should either have the declaration scopes shadow all the import scopes, or produce a conflict.

What I think would be a simple mental model here, is to make most things be conflicts. The only shadowing that would happen is the current files declaration scope would be allowed to shadow the import scope (both inherited and current). No other shadowing at all would happen, everything else is a conflict.

Essentially, stuff in the pure lexical scope can shadow stuff in the import scope, and nothing else can.

lrhn commented 1 month ago

Each file contains declared import directives, some prefixed, and (top-level) declaration. Those are what we can combine to get the top-level scopes of every file.

For backwards compatibility, we want a top-level scope of each file which can contain, and will if no part file has any imports:

Anything that would break this is a no-go, it would break existing libraries with part files.

That means that that's going to be the top-level scope of the library file, and the default scope of a first-level part file with no imports will be compatible with that (resolve names to the same declarations, whether its structured the same way or not).

We want consistency, so adding futher nested part files with no imports should still give compatible top-level scopes in every file.

Our options are limited to what adding imports in a part file changes its top-level scope, and those if its sub-parts.

When you say

You can't shadow declarations from other parts of the library.

then because all those declarations are available as default, it means that the name should still be accessible in every file, no matter what you do with imports.

Importing something with the same name as a declaration anywhere in the library will not shadow that declaration. It also shouldn't be an error, because that's breaking, and very fragile.

Which leaves us at each library having all the declarations of the entire library in their top-level scope, shadowing any imports.

Your imports can't shadow declarations from inherited imports.

That's new and possible, since no part files have imports yet. What it means depends on what you actually inherit, but it's probably doable in any case.

It means that importing something with the same name as an inherited import, will make the name conflicted. If you never use it, that's not a problem. If your declarations shadow it, it'll never be a problem.

Declarations from other parts (including the root library) can't shadow declarations from inherited imports or current file's imports.

I think that's inconsistent with requirements and breaking, and I don't think that'll ever work.

Every declaration of every file is in the top-level scope, they will shadow imports. The only way to have this restriction is that it's a compile-time error to have a declaration in any file of the library (and therefore every file of the library) with the same name as any imported declaration in any file of the library.

Far too breaking. Imports tend to over-import, and if the library declares something with the same name, it has always just taken precedence, and made the imported name inaccessible.

The only shadowing that would happen is the current files declaration scope would be allowed to shadow the import scope (both inherited and current).

You can't shadow inherited imports. The declartions of the current files are also in the scope of the parent file that the import came from, and would conflict there. The only thing you can possibly shadow is imports in the same file as the declaration. And again, it would be breaking if a declaration in a part file cannot have the same name as an import in the library file.

I think shadowing should be allowed, because it's actually useful. I generally see no advantage in disallowing shadowing or causing errors. The exact details about which declarations go in which scope order for part file imports can be twiddled with, but just treating everything like one big scope, where same-names conflict, is removing some of the flexibility that part-files with imports can have.

(One alternative is to say that all import declarations in all files are applied in the library file, as its import scope, and its top-level scope is that extended with every declaration in the library. And then use that scope for every part file. But part of the reason for the design here was to give part files, fx generated by macros, the ability to safely control their scope, without risking introducing conflicts with any code anywhere else in the same library.)

jakemac53 commented 1 month ago

I strongly disagree that we cannot make a breaking change here. We have language versioning, and are already intending on using it. Nobody will be broken until they upgrade their language version, and we should be able to provide a dart fix as well.

I also do not anticipate that the suggested semantics would cause much breakage. It only matters if a library is using parts, and shadows a declaration from its imports, inside a part file.

  • The declarations of every file in the library, shadowing imports of the library file.

I fundamentally do not think it is a good property to have. These declarations are not in the lexical scope, and so the main user intuition about scoping is gone. Beyond the lexical scope, I don't think users have a good intuition about how shadowing works at all. Thus, it is better to make the choice explicit.

As a concrete example, I think it is highly confusing today that the import scope shadows the super member scope (I don't know what we actually call this scope). This should be a conflict, and require a this. or <prefix>. disambiguation. I do not believe your average user understands how this works (probably much of the Dart team doesn't!).

I would like to avoid similar issues, hence allowing the lexical scope to shadow the import scope, and everything else being a conflict.

Consider also that the "parent" and "children" in this case are also directly included with URI based directives, in the same way as imports. Essentially, the proposal is to put things from all URI based directives in the same scope. They are things which are external to the current file, and are all treated equally in terms of the scope. The only difference is whether they also include their private scope, and whether you are allowed to augment the declarations from those files.

Concretely:

Parent's inherited frames
Parent import scope w/conflicts (parents imports)
Own import scope w/conflicts (own imports)
Parent import prefix scope w/conflicts (parents prefixed import names)
Own import prefix scope w/conflicts (own prefixed import names)
Childrens declaration scope w/conflicts part declarations
Parents declaration scope w/conflicts part of declarations
Own declaration scope Only current files declarations

You could re-order as you please any of this except the own declaration scope, because it's conflicts all the way.

Technically, we could even allow prefixes for part of and part, to be fully consistent.

lrhn commented 1 month ago

the super member scope (I don't know what we actually call this scope)

We don't call it anything, because it's not a scope.

If a raw identifier, id, is not in the lexical scope (which is the entire scope chain, including imports and declaration scope, not just declarations syntactically in the same file), and it occurs inside instance member code, the identifier is an instance member access equivalent to this.id.

This should be a conflict, and require a this. or <prefix>. disambiguation.

The one thing that keeps being said about this weirdness is that it causes surprisingly few problems. In most cases, it just works, and when it doesn't, you can add a this. to get the effect you want. There isn't always a prefix you can add to get to the non-instance declaration, which is why that being the default works out better than the opposite.

That said, I'd be willing to look at only allowing declarations in the current file to win over an instance member. An import doesn't count unless it has a show clause with the name. A declaration in another file of the library ... is exactly the case where there is no syntax to choose it over this._id. We'd have to add something to allow you to access that name. (Maybe you can add an abstract declaration in the current library? Or is that to .c/.h-like?)

It would mean having to look up every plain identifier that resolves to an import name in the current class, to check if it has that name (can probably be cached, if the same name is used a lot). Compiling any function doing print(..) or identical(..), the compiler needs to check if the surrounding class has a print or identical member, where today it can just say "found it!" when it resolves to the import from dart:core.

These declarations are not in the lexical scope,

They are, today, because we've defined the lexical scope so that they are, the import scope and declaration scope are at the top of every lexically induced scope. (There is no difference between being "in scope" and being in the lexical scope. We always say explicitly if we use any other scope than the lexical scope, the "lexical" is mainly for emphasis.)

The issue here is that there are declarations that are not syntactically visible in the file, even though they are in the lexical scope, and people tend to think of the lexical scope as only what they can see above. Imports can be seen in the library file, by their import directive, even if you can't see the names they include, but top-level declarations from other files are completely invisible. And imports are invisible in part files too.

I'm not sure I agree that that's all bad. I works remarkably well, if anything, because it doesn't get in the users way most of the time. If a declaration conflicting with an import is an error, I'd have to add a lot of show clauses, to solve a problem I don't have, so it's just a hurdle introduced by the language. Don't see that making anyone happy.

For the scope chain: Which declarations are in the "Parents declaration scope w/conflicts"? Is it only declarations physically in the parent file, or also declarations it includes from its sub-parts or parents? (The reason I ordered the frames as "Parent's ..." then "Own ..." was that that really is the scope chain. If you move "parent's declaration scope" down, giving it a different parent scope, it's not longer the parent's declaration scope, so it's not clear which declarations are in it.)

jakemac53 commented 1 month ago

Ah, sorry I didn't realize how we defined the lexical scope. I always think of it only as applying to what is visibly (textually) surrounding a given location in the code. So, this is what I was referring to in all my comments above.

I think people understand/expect identifiers to resolve to the "closest" declaration in the actual text surrounding that identifier. But as soon as it isn't found anywhere in that text, they don't have a good intuition about where it is looked up, in particular when it comes to the edge cases.

We can certainly specify something that provides a consistent result, but it is likely that for some subset of users that won't be the intuitive result, regardless of the choice we make, because they won't all have the same intuition.

The one thing that keeps being said about this weirdness is that it causes surprisingly few problems.

When it does cause a problem though, it can be extremely confusing. We could also use this argument as evidence that it would be perfectly fine to require explicit disambiguation here, because it will rarely actually be required.

Compiling any function doing print(..) or identical(..), the compiler needs to check if the surrounding class has a print or identical member, where today it can just say "found it!" when it resolves to the import from dart:core.

True, it would mean some extra work. And a similar argument could be made for my proposal here - we could no longer search just the whole surrounding library for declarations, and immediately return if we find one. You have to also search all imports for any conflicting declarations, if it was found in a different "part" of the library. It feels like we must already have a pretty optimized way of doing this kind of lookup though.

I'm not sure I agree that that's all bad. I works remarkably well, if anything, because it doesn't get in the users way most of the time. If a declaration conflicting with an import is an error, I'd have to add a lot of show clauses, to solve a problem I don't have, so it's just a hurdle introduced by the language. Don't see that making anyone happy.

It is probably worth collecting some data here - I think you and me have a different intuition for how much churn this would actually cause. My intuition is it would be very little churn.

Essentially the question to answer would be, within a given library how many identifiers resolve to a declaration in that library which is shadowing a declaration by the same name from an import.

If that number is large (even say 10% of libraries have a single one of these occurrences), then I would likely agree it is more annoying than it is worth.

If the number turns out to be very small, let's say <1% of all libraries, would you be willing to consider my proposal, or would you still consider it too annoying? Is there a different number you would be happy with?

Keep in mind, the actual effect of this would be smaller than that number, because it's only for part files where a prefix would actually be needed.

For the scope chain: Which declarations are in the "Parents declaration scope w/conflicts"? Is it only declarations physically in the parent file, or also declarations it includes from its sub-parts or parents? (The reason I ordered the frames as "Parent's ..." then "Own ..." was that that really is the scope chain. If you move "parent's declaration scope" down, giving it a different parent scope, it's not longer the parent's declaration scope, so it's not clear which declarations are in it.)

That makes sense, so I think it just becomes:

Parent's inherited frames
...Parents scope part of's declarations, imports, child declarations, parent scope
Children's declaration scope w/conflicts part declarations
Own import scope w/conflicts (own imports)
Own declaration scope Only current files declarations

And then that answers both questions? The children's declaration scope is a bit weird to slot in, you need to skip it if it's already included in the chain previously.

jakemac53 commented 3 weeks ago

I wrote an analyzer lint to check this, which triggers any time any declaration in a library shadows one from an import, and I ran it on the flutter repo, which has 116 total violations of the lint:

All violations ``` info - dev/a11y_assessments/test/accessibility_guideline_test.dart:10:1 - The declaration 'main' shadows a declaration from the import package:a11y_assessments/main.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/benchmarks/macrobenchmarks/lib/src/cubic_bezier.dart:106:1 - The declaration 'Point' shadows a declaration from the import dart:math. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/benchmarks/macrobenchmarks/lib/src/web/material3.dart:153:1 - The declaration 'Actions' shadows a declaration from the import package:flutter/material.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/benchmarks/macrobenchmarks/test_driver/large_image_changer.dart:13:1 - The declaration 'main' shadows a declaration from the import package:macrobenchmarks/main.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/benchmarks/macrobenchmarks/test_memory/heavy_gridview.dart:17:1 - The declaration 'main' shadows a declaration from the import package:macrobenchmarks/main.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/benchmarks/macrobenchmarks/test_memory/large_images.dart:17:1 - The declaration 'main' shadows a declaration from the import package:macrobenchmarks/main.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/benchmarks/microbenchmarks/lib/stocks/build_bench_profiled.dart:11:1 - The declaration 'main' shadows a declaration from the import package:microbenchmarks/stocks/build_bench.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/bots/service_worker_test.dart:180:1 - The declaration 'expect' shadows a declaration from the import~/flutter/dev/bots/test/common.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/bots/test/analyze_test.dart:44:1 - The declaration 'main' shadows a declaration from the import~/flutter/dev/bots/analyze.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/bots/test/check_code_samples_test.dart:15:1 - The declaration 'main' shadows a declaration from the import~/flutter/dev/bots/check_code_samples.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/bots/test/codesign_test.dart:12:1 - The declaration 'main' shadows a declaration from the import~/flutter/dev/bots/test.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/bots/test/common.dart:11:1 - The declaration 'isInstanceOf' shadows a declaration from the import package:test/test.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/bots/test/post_process_docs_test.dart:14:1 - The declaration 'main' shadows a declaration from the import~/flutter/dev/bots/post_process_docs.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/bots/test/test_test.dart:34:1 - The declaration 'main' shadows a declaration from the import~/flutter/dev/bots/test.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/conductor/core/test/packages_autoroller_test.dart:17:1 - The declaration 'main' shadows a declaration from the import~/flutter/dev/conductor/core/bin/packages_autoroller.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/devicelab/bin/tasks/build_aar_module_test.dart:13:14 - The declaration 'platformLineSep' shadows a declaration from the import package:flutter_devicelab/framework/apk_utils.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/devicelab/bin/tasks/module_custom_host_app_name_test.dart:16:14 - The declaration 'platformLineSep' shadows a declaration from the import package:flutter_devicelab/framework/apk_utils.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/devicelab/bin/tasks/module_test.dart:20:14 - The declaration 'platformLineSep' shadows a declaration from the import package:flutter_devicelab/framework/apk_utils.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/devicelab/lib/framework/utils.dart:57:1 - The declaration 'ProcessInfo' shadows a declaration from the import dart:io. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/devicelab/lib/framework/utils.dart:704:1 - The declaration 'jsonEncode' shadows a declaration from the import dart:convert. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/devicelab/test/adb_test.dart:181:1 - The declaration 'FakeDevice' shadows a declaration from the import package:flutter_devicelab/framework/devices.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/devicelab/test/common.dart:9:1 - The declaration 'isInstanceOf' shadows a declaration from the import package:test/test.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/integration_tests/channels/integration_test/main_test.dart:14:1 - The declaration 'main' shadows a declaration from the import package:channels/main.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/integration_tests/new_gallery/lib/pages/about.dart:11:1 - The declaration 'showAboutDialog' shadows a declaration from the import package:flutter/material.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/integration_tests/new_gallery/lib/routes.dart:27:1 - The declaration 'Path' shadows a declaration from the import package:flutter/material.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/integration_tests/new_gallery/test/pages/home_test.dart:9:1 - The declaration 'main' shadows a declaration from the import package:gallery/main.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/integration_tests/new_gallery/test/widget_test.dart:8:1 - The declaration 'main' shadows a declaration from the import package:gallery/main.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/integration_tests/new_gallery/test_driver/transitions_perf.dart:29:1 - The declaration 'main' shadows a declaration from the import package:gallery/main.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/integration_tests/spell_check/integration_test/integration_test.dart:50:1 - The declaration 'main' shadows a declaration from the import package:spell_check/main.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/tools/create_api_docs.dart:78:1 - The declaration 'main' shadows a declaration from the import~/flutter/dev/tools/dartdoc_checker.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/tools/test/create_api_docs_test.dart:15:1 - The declaration 'main' shadows a declaration from the import~/flutter/dev/tools/dartdoc_checker.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - dev/tools/test/update_icons_test.dart:24:1 - The declaration 'main' shadows a declaration from the import~/flutter/dev/tools/update_icons.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - examples/api/lib/widgets/transitions/listenable_builder.3.dart:50:1 - The declaration 'ListBody' shadows a declaration from the import package:flutter/material.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - examples/api/test/painting/image_provider/image_provider.0_test.dart:9:1 - The declaration 'main' shadows a declaration from the import package:flutter_api_samples/painting/image_provider/image_provider.0.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - examples/api/test/rendering/box/parent_data.0_test.dart:9:1 - The declaration 'main' shadows a declaration from the import package:flutter_api_samples/rendering/box/parent_data.0.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - examples/api/test/widgets/scroll_view/grid_view.0_test.dart:9:1 - The declaration 'main' shadows a declaration from the import package:flutter_api_samples/widgets/scroll_view/grid_view.0.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - examples/layers/test/gestures_test.dart:10:1 - The declaration 'main' shadows a declaration from the import~/flutter/examples/layers/widgets/gestures.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - examples/layers/test/sector_test.dart:10:1 - The declaration 'main' shadows a declaration from the import~/flutter/examples/layers/widgets/sectors.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter/lib/src/material/refresh_indicator.dart:32:1 - The declaration 'RefreshCallback' shadows a declaration from the import package:flutter/cupertino.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter/lib/src/scheduler/binding.dart:40:1 - The declaration 'FrameCallback' shadows a declaration from the import dart:ui. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter/test/examples/sector_layout_test.dart:13:1 - The declaration 'main' shadows a declaration from the import~/flutter/examples/layers/rendering/custom_coordinate_systems.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter/test/gestures/gesture_binding_resample_event_test.dart:14:1 - The declaration 'HandleEventCallback' shadows a declaration from the import package:flutter/gestures.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter/test/gestures/gesture_binding_test.dart:12:1 - The declaration 'HandleEventCallback' shadows a declaration from the import package:flutter/gestures.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter/test/gestures/locking_test.dart:11:1 - The declaration 'HandleEventCallback' shadows a declaration from the import package:flutter/gestures.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter/test/material/ink_splash_test.dart:9:1 - The declaration 'Page' shadows a declaration from the import package:flutter/material.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter/test/material/mergeable_material_test.dart:14:1 - The declaration 'matches' shadows a declaration from the import package:flutter_test/flutter_test.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter/test/material/time_picker_test.dart:2204:1 - The declaration 'MaterialType' shadows a declaration from the import package:flutter/material.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter/test/rendering/proxy_box_test.dart:1093:1 - The declaration 'DebugPaintCallback' shadows a declaration from the import package:flutter/rendering.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter/test/scheduler/scheduler_tester.dart:7:1 - The declaration 'Future' shadows a declaration from the import dart:core. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter/test/services/restoration_test.dart:372:1 - The declaration 'TestRestorationManager' shadows a declaration from the import package:flutter_test/flutter_test.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter/test/widgets/dispose_ancestor_lookup_test.dart:9:1 - The declaration 'TestCallback' shadows a declaration from the import package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter/test/widgets/heroes_test.dart:16:1 - The declaration 'createTestImage' shadows a declaration from the import package:flutter_test/flutter_test.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter/test/widgets/independent_widget_layout_test.dart:267:1 - The declaration 'WidgetState' shadows a declaration from the import package:flutter/material.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter/test/widgets/slivers_block_global_key_test.dart:27:1 - The declaration 'test' shadows a declaration from the import package:flutter_test/flutter_test.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter/test/widgets/slivers_block_test.dart:9:1 - The declaration 'test' shadows a declaration from the import package:flutter_test/flutter_test.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter/test/widgets/slivers_padding_test.dart:21:1 - The declaration 'test' shadows a declaration from the import package:flutter_test/flutter_test.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter/test/widgets/slivers_test.dart:12:1 - The declaration 'test' shadows a declaration from the import package:flutter_test/flutter_test.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter/test/widgets/slivers_test.dart:1545:1 - The declaration 'KeepAlive' shadows a declaration from the import package:flutter/material.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_test/lib/src/matchers.dart:299:1 - The declaration 'isInstanceOf' shadows a declaration from the import package:matcher/expect.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_test/lib/src/test_compat.dart:108:1 - The declaration 'test' shadows a declaration from the import package:test_api/scaffolding.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_test/lib/src/test_compat.dart:179:1 - The declaration 'group' shadows a declaration from the import package:test_api/scaffolding.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_test/lib/src/test_compat.dart:194:1 - The declaration 'setUp' shadows a declaration from the import package:test_api/scaffolding.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_test/lib/src/test_compat.dart:210:1 - The declaration 'tearDown' shadows a declaration from the import package:test_api/scaffolding.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_test/lib/src/test_compat.dart:228:1 - The declaration 'setUpAll' shadows a declaration from the import package:test_api/scaffolding.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_test/lib/src/test_compat.dart:246:1 - The declaration 'tearDownAll' shadows a declaration from the import package:test_api/scaffolding.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_test/test/reference_image_test.dart:11:1 - The declaration 'createTestImage' shadows a declaration from the import package:flutter_test/flutter_test.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/lib/src/convert.dart:23:1 - The declaration 'Utf8Codec' shadows a declaration from the import dart:convert. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/lib/src/convert.dart:47:1 - The declaration 'Utf8Decoder' shadows a declaration from the import dart:convert. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/lib/src/globals.dart:63:1 - The declaration 'deviceManager' shadows a declaration from the import package:flutter_tools/src/device.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/lib/src/test/flutter_platform.dart:275:1 - The declaration 'Finalizer' shadows a declaration from the import dart:core. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/commands.shard/hermetic/build_ios_test.dart:1439:1 - The declaration 'FakeAndroidSdk' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/commands.shard/hermetic/build_ios_test.dart:1450:1 - The declaration 'FakeOperatingSystemUtils' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/commands.shard/hermetic/clean_test.dart:252:1 - The declaration 'FakeXcodeProjectInterpreter' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/context.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/commands.shard/hermetic/daemon_test.dart:1266:1 - The declaration 'FakeDeviceLogReader' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fake_devices.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/commands.shard/hermetic/doctor_test.dart:1132:1 - The declaration 'FakeDoctor' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/context.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/commands.shard/hermetic/doctor_test.dart:1404:1 - The declaration 'FakeDeviceManager' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/context.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/commands.shard/hermetic/downgrade_test.dart:235:1 - The declaration 'FakeStdio' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/commands.shard/hermetic/drive_test.dart:711:1 - The declaration 'FakeSignals' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/context.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/commands.shard/hermetic/proxied_devices_test.dart:352:1 - The declaration 'FakeDeviceLogReader' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fake_devices.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/commands.shard/hermetic/run_test.dart:1210:1 - The declaration 'FakeDevice' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fake_devices.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/commands.shard/permeable/build_aar_test.dart:316:1 - The declaration 'FakeAndroidBuilder' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/android_common.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/commands.shard/permeable/build_aar_test.dart:339:1 - The declaration 'FakeAndroidSdk' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/commands.shard/permeable/build_aar_test.dart:342:1 - The declaration 'FakeAndroidStudio' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/commands.shard/permeable/build_apk_test.dart:475:1 - The declaration 'FakeAndroidSdk' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/commands.shard/permeable/build_apk_test.dart:482:1 - The declaration 'FakeAndroidStudio' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/commands.shard/permeable/build_appbundle_test.dart:252:1 - The declaration 'FakeAndroidSdk' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/commands.shard/permeable/devices_test.dart:84:1 - The declaration 'FakeDeviceManager' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/context.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/analytics_test.dart:387:1 - The declaration 'FakeDoctor' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/context.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/android/android_device_discovery_test.dart:264:1 - The declaration 'FakeAndroidSdk' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/android/android_gradle_builder_test.dart:2001:1 - The declaration 'FakeAndroidStudio' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/android/android_project_migration_test.dart:451:1 - The declaration 'FakeAndroidStudio' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/android/android_workflow_test.dart:619:1 - The declaration 'FakeAndroidSdk' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/application_package_test.dart:941:1 - The declaration 'FakeOperatingSystemUtils' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/application_package_test.dart:950:1 - The declaration 'FakeAndroidSdk' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/artifact_updater_test.dart:531:1 - The declaration 'FakeOperatingSystemUtils' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/base/logger_test.dart:26:1 - The declaration 'main' shadows a declaration from the import package:flutter_tools/executable.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/base/terminal_test.dart:302:1 - The declaration 'FakeStdio' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/cache_test.dart:1261:1 - The declaration 'FakeAndroidSdk' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/compile_expression_test.dart:181:1 - The declaration 'FakeProcess' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fake_process_manager.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/compile_expression_test.dart:195:1 - The declaration 'FakeProcessManager' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fake_process_manager.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/device_test.dart:1164:1 - The declaration 'TestDeviceDiscoverySupportFilter' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/context.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/emulator_test.dart:402:1 - The declaration 'FakeAndroidSdk' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/flutter_validator_test.dart:612:1 - The declaration 'FakeOperatingSystemUtils' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/ios/devices_test.dart:1053:1 - The declaration 'FakeProcess' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fake_process_manager.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/ios/xcode_debug_test.dart:1245:1 - The declaration 'FakeProcess' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fake_process_manager.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/linux/linux_device_test.dart:180:1 - The declaration 'FakeOperatingSystemUtils' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/macos/cocoapods_test.dart:1382:1 - The declaration 'FakeXcodeProjectInterpreter' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/context.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/macos/xcode_test.dart:1678:1 - The declaration 'FakeXcodeProjectInterpreter' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/context.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/plugins_test.dart:1492:1 - The declaration 'FakeXcodeProjectInterpreter' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/context.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/project_test.dart:1726:1 - The declaration 'FakeXcodeProjectInterpreter' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/context.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/runner/flutter_command_runner_test.dart:326:1 - The declaration 'FakeStdio' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/fakes.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart:1327:1 - The declaration 'FakeSignals' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/context.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/runner/target_devices_test.dart:3003:1 - The declaration 'FakeDoctor' shadows a declaration from the import~/flutter/packages/flutter_tools/test/src/context.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/general.shard/xcode_backend_test.dart:13:1 - The declaration 'main' shadows a declaration from the import~/flutter/packages/flutter_tools/bin/xcode_backend.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/flutter_tools/test/integration.shard/transition_test_utils.dart:16:22 - The declaration 'processManager' shadows a declaration from the import~/flutter/packages/flutter_tools/test/integration.shard/test_utils.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations info - packages/integration_test/lib/integration_test_driver_extended.dart:22:8 - The declaration 'testOutputsDirectory' shadows a declaration from the import package:flutter_driver/flutter_driver.dart. Try hiding the declaration from the import, or importing it with a prefix. - avoid_shadowing_import_declarations ```

Many of these cases are main functions, some are also just straight up duplicate code, I didn't really see any where I personally felt that the code would be worse off needing an explicit hide or prefix.

However, the potential for breakage when adding new top level symbols to any package is definitely worth consideration. I didn't fully analyze how many of these were shadowing things from different packages, which is the main concern there (within the same package it wouldn't matter).

I will see if I can also run this internally, and maybe on some actual apps as well, although it is a bit hard to get successful pub solves in the external apps I was looking at, each one needs some different version of flutter it seemed like. In general needing full analysis to produce this diagnostic makes it hard to run on a large corpus of packages/apps.

jakemac53 commented 2 weeks ago

Unfortunately I wasn't able to get the lint to apply correctly internally, not sure why 🤷‍♂️ . But, I think I have seen enough to be convinced that what you originally proposed is OK. It probably will do what people expect most of the time. I think ultimately I just wish shadowing wasn't a thing at all, but I am outlier in that I think.

This does double down on shadowing, adding more layers of scopes and more places shadowing can happen, but 🤷‍♂️ .

jakemac53 commented 1 week ago

I am going to go ahead and close this one as well