dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.23k stars 1.57k forks source link

Fix sort order of folders in options map to ensure most specific lookups #55252

Closed pq closed 7 months ago

pq commented 7 months ago

Enabling multi-option support causes some bot failures.

https://logs.chromium.org/logs/dart/buildbucket/cr-buildbucket/8754154758524980737/+/u/analyze_pkg_/stdout

At least 2 issues remain unresolved:

  1. There are a host of newly reported diagnostics. For example:

info - vm/testcases/transformations/ffi/finalizable_late_2.dart:19:31 - Statements in a for should be enclosed in a block. Try wrapping the statement in a block. - curly_braces_in_flow_control_structures

curly_braces_in_flow_control_structures should not be enabled here. It's a lint enabled by the core set which is included in an options file in the parent directory but without an include here should not be inherited (unless we want to change current semantics).

Here's the relevant options file:

analyzer:
  errors:
    sdk_version_since: ignore
  exclude:
    - deferred_loading/**
    - to_string_transformer/**
    - type_flow/**
    - unreachable_code_elimination/**

Note no include:.

  1. Error severity configurations may not be working right.

Looking above, you'd expect sdk_version_since to be filtered, but it's not:

warning - vm/testcases/transformations/ffi/ffinative.dart:72:10 - This API is available since SDK 3.3.0, but constraints '>=3.0.0 <4.0.0' don't guarantee it. Try updating the SDK constraints. - sdk_version_since

pq commented 7 months ago

A fresh run of dart analyze --fatal-infos pkg with multi-option contexts shows these diagnostics:

Analyzing pkg...

warning - compiler/test/codesize/swarm/ConfigHintDialog.dart:10:17 - The value of the field '_parent' isn't used. Try removing the field, or using it. - unused_field
warning - compiler/test/codesize/swarm/DataSource.dart:106:16 - The value of the local variable 'feed' isn't used. Try removing the variable or using it. - unused_local_variable
warning - compiler/test/codesize/swarm/DataSource.dart:196:12 - The value of the local variable 'home' isn't used. Try removing the variable or using it. - unused_local_variable
warning - compiler/test/codesize/swarm/DataSource.dart:212:19 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/HelpDialog.dart:12:23 - The value of the field '_parent' isn't used. Try removing the field, or using it. - unused_field
warning - compiler/test/codesize/swarm/SwarmApp.dart:31:21 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/SwarmApp.dart:70:30 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/SwarmState.dart:90:50 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/SwarmState.dart:95:50 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/SwarmViews.dart:712:11 - The value of the local variable 'metrics' isn't used. Try removing the variable or using it. - unused_local_variable
warning - compiler/test/codesize/swarm/SwarmViews.dart:725:24 - The operand can't be null, so the condition is always 'false'. Try removing the condition, an enclosing condition, or the whole conditional statement. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/SwarmViews.dart:787:24 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/Views.dart:337:13 - The value of the local variable 'snappedCurrentPosition' isn't used. Try removing the variable or using it. - unused_local_variable
warning - compiler/test/codesize/swarm/Views.dart:659:20 - The operand can't be null, so the condition is always 'false'. Try removing the condition, an enclosing condition, or the whole conditional statement. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/Views.dart:748:7 - The value of the field '_lastOffset' isn't used. Try removing the field, or using it. - unused_field
warning - compiler/test/codesize/swarm/Views.dart:750:14 - The value of the field '_paginate' isn't used. Try removing the field, or using it. - unused_field
warning - compiler/test/codesize/swarm/Views.dart:891:30 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/Views.dart:893:30 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/layout/GridLayout.dart:383:21 - The operand can't be null, so the condition is always 'false'. Try removing the condition, an enclosing condition, or the whole conditional statement. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/layout/GridLayout.dart:417:12 - The value of the local variable 'left' isn't used. Try removing the variable or using it. - unused_local_variable
warning - compiler/test/codesize/swarm/swarm_ui_lib/layout/GridLayout.dart:417:31 - The value of the local variable 'width' isn't used. Try removing the variable or using it. - unused_local_variable
warning - compiler/test/codesize/swarm/swarm_ui_lib/layout/GridLayout.dart:418:12 - The value of the local variable 'top' isn't used. Try removing the variable or using it. - unused_local_variable
warning - compiler/test/codesize/swarm/swarm_ui_lib/layout/GridLayout.dart:418:30 - The value of the local variable 'height' isn't used. Try removing the variable or using it. - unused_local_variable
warning - compiler/test/codesize/swarm/swarm_ui_lib/layout/GridLayoutParser.dart:10:16 - The value of the field 'WHITESPACE' isn't used. Try removing the field, or using it. - unused_field
warning - compiler/test/codesize/swarm/swarm_ui_lib/layout/GridLayoutParser.dart:172:10 - The value of the local variable 'dot' isn't used. Try removing the variable or using it. - unused_local_variable
warning - compiler/test/codesize/swarm/swarm_ui_lib/layout/GridTracks.dart:97:13 - The value of the field '_numRows' isn't used. Try removing the field, or using it. - unused_field
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/ClickBuster.dart:158:22 - The operand can't be null, so the condition is always 'false'. Try removing the condition, an enclosing condition, or the whole conditional statement. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/FxUtil.dart:64:11 - The value of the local variable 'testPoint' isn't used. Try removing the variable or using it. - unused_local_variable
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/Momentum.dart:336:19 - The value of the field '_minCoord' isn't used. Try removing the field, or using it. - unused_field
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/Momentum.dart:337:19 - The value of the field '_maxCoord' isn't used. Try removing the field, or using it. - unused_field
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/ScrollWatcher.dart:23:12 - The value of the field '_scrollerEl' isn't used. Try removing the field, or using it. - unused_field
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/Scrollbar.dart:87:26 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/Scrollbar.dart:302:18 - The operand can't be null, so the condition is always 'false'. Try removing the condition, an enclosing condition, or the whole conditional statement. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/Scroller.dart:158:13 - The value of the local variable 'parentElem' isn't used. Try removing the variable or using it. - unused_local_variable
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/Scroller.dart:282:27 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/Scroller.dart:369:24 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/Scroller.dart:386:28 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/Scroller.dart:392:26 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/Scroller.dart:397:25 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/Scroller.dart:425:28 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/Scroller.dart:451:13 - The value of the local variable 'touch' isn't used. Try removing the variable or using it. - unused_local_variable
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/Scroller.dart:488:38 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/Scroller.dart:517:25 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/Scroller.dart:569:22 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/Scroller.dart:570:22 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/TouchHandler.dart:93:12 - The value of the field '_startTime' isn't used. Try removing the field, or using it. - unused_field
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/TouchHandler.dart:195:23 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/TouchHandler.dart:208:34 - The operand can't be null, so the condition is always 'false'. Try removing the condition, an enclosing condition, or the whole conditional statement. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/TouchHandler.dart:231:34 - The operand can't be null, so the condition is always 'false'. Try removing the condition, an enclosing condition, or the whole conditional statement. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/TouchHandler.dart:282:51 - The operand can't be null, so the condition is always 'false'. Try removing the condition, an enclosing condition, or the whole conditional statement. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/touch/touch.dart:14:9 - The library 'dart:collection' doesn't export a member with the shown name 'ImmutableListMixin'. Try removing the name from the list of shown members. - undefined_shown_name
warning - compiler/test/codesize/swarm/swarm_ui_lib/util/CollectionUtils.dart:100:11 - The value of the local variable 'iter' isn't used. Try removing the variable or using it. - unused_local_variable
warning - compiler/test/codesize/swarm/swarm_ui_lib/view/CompositeView.dart:65:20 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/view/CompositeView.dart:73:19 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/view/CompositeView.dart:85:19 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
warning - compiler/test/codesize/swarm/swarm_ui_lib/view/PagedViews.dart:228:11 - The value of the local variable 'destination' isn't used. Try removing the variable or using it. - unused_local_variable
warning - compiler/test/codesize/swarm/swarm_ui_lib/view/view.dart:97:17 - The operand can't be null, so the condition is always 'true'. Remove the condition. - unnecessary_null_comparison
   info - compiler/test/codesize/swarm/SwarmViews.dart:816:8 - The declaration '_saveToStorage' isn't referenced. Try removing the declaration of '_saveToStorage'. - unused_element
   info - compiler/test/codesize/swarm/swarm_ui_lib/touch/Scroller.dart:580:8 - The declaration '_stopDecelerating' isn't referenced. Try removing the declaration of '_stopDecelerating'. - unused_element
   info - compiler/test/codesize/swarm/swarm_ui_lib/touch/TouchUtil.dart:112:6 - The declaration '_touchEventTargetsNode' isn't referenced. Try removing the declaration of '_touchEventTargetsNode'. - unused_element

60 issues found.

https://logs.chromium.org/logs/dart/buildbucket/cr-buildbucket/8752912285242474017/+/u/analyze_pkg_/stdout

Whereas running with a single options file per context yields none.

dart analyze --fatal-infos pkg
Analyzing pkg...                       78.0s
No issues found!
pq commented 7 months ago

On the surface, it looks like the error filters are not getting properly applied.

test/codesize/swarm/swarm_ui_lib/analysis_options.yaml defines the following ignores, which should silence the errors reported above...

analyzer:
  errors:
    todo: ignore
    deprecated_member_use: ignore
    illegal_language_version_override: ignore
    unused_local_variable: ignore
    unused_field: ignore
    unused_element: ignore
    unnecessary_null_comparison: ignore
    undefined_shown_name: ignore
pq commented 7 months ago

Interestingly, reproduction depends on being run over the pkg directory. Analyzing just the compiler (dart analyze --fatal-infos compiler) runs clean.

pq commented 7 months ago

UPDATE: the issue seems to be in how we're sorting folders to improve options lookup speed.


Specifically, in local testing I was able to confirm that lookup for compiler/test/codesize/swarm/ConfigHintDialog.dart was returning the options defined in compiler/test rather than the more specific ones defined in compiler/test/codesize/swarm (or compiler/test).