Fix sample analysis build jobs #2575

kwalrath · 2020-08-13T02:18:34Z

I had to disregard sample analysis (see f5af1bd) to fix the build when 2.9 stable was released.

But the site should always pass static analysis in the stable build, and preferably the beta build as well (#2525).

I'm uploading my work in progress, so @theacodes can (I hope) take it over and actually fix the build.

I'm not sure if any of these are used in the docs... but should we get the same results?

kwalrath · 2020-08-13T16:01:36Z

Don't let the Travis build passing fool you... analysis still fails. (I should've updated .travis.yml.) Here's how it fails now: https://travis-ci.org/github/dart-lang/site-www/jobs/717457700.

This seemed to be broken becuase of a deleted/renamed file (`animal_bad.dart` & `common_problems_analysis.dart`).

kwalrath · 2020-08-13T16:46:36Z

The reason the build is failing is, apparently, that errors can no longer be ignored. There's a discussion at dart-lang/sdk#42977.

These are existing warnings that we'd ignored that became unsilenceable errors in 2.9

…hat incorrectly ignores expected analysis-results files on travis

…ed these from has been removed due to it being invalid code in 2.9

…line

…for examples/strong

theacodes · 2020-08-13T19:52:08Z

It ain't easy being green. 🐸

kwalrath

Some questions, and some places where I'd like to consult with people who've been working on them (Bob for Effective Dart, John for the site infra), but I'm so happy to have this running again!

kwalrath · 2020-08-13T20:04:55Z

examples/misc/lib/library_tour/core/collections.dart

@@ -1,6 +1,6 @@
 void miscDeclAnalyzedButNotTested() {
  var fruits = List<String>();
-  // ignore_for_file: stable, dev, argument_type_not_assignable
+  // ignore_for_file: stable, dev, argument_type_not_assignable, unused_local_variable


Should we add beta to this list? Or remove the release names?

(Is "beta" even supported? I don't see examples/strong/analyzer-results-beta.txt in this PR.)

Not sure? I can't quite figure out how that works. If there isn't a -beta.txt file it uses analyzer-results.txt, and I think most of the beta files that existed were fine.

kwalrath · 2020-08-13T20:06:48Z

examples/strong/analyzer-results-dev.txt

@@ -1,18 +1,11 @@
 Analyzing analysis_options.yaml, lib, test...
-  error • 'HoneyBadger.parent' ('Root Function()') isn't a valid override of 'Animal.parent' ('Animal Function()'). • lib/animal_bad.dart:15:12 • invalid_override


Is this file different from analyzer-results-stable.txt? If not, then maybe we should remove one of these files and delete "-stable" from the name.

It's identical.

kwalrath · 2020-08-13T20:07:49Z

src/_guides/language/effective-dart/design.md

@@ -1281,8 +1281,10 @@ num highScore(List<num> scores) {
 }
 {% endprettify %}

+<!-- Q: As of 2.9, this code no longer works.


@munificent opinion?

Why doesn't it work? Because we're getting rid of implicit downcasts?

Why doesn't it work? Because we're getting rid of implicit downcasts?

Yes, I think so. Here's the message, which complains about highest (an int) being assigned score (a num).

error * A value of type 'num' can't be assigned to a variable of type 'int'. * lib/effective_dart/design_bad.dart:79:38 * invalid_assignment

num highScore(List<num> scores) { var highest = 0; for (var score in scores) { // ignore: invalid_assignment if (score > highest) highest = score; } return highest; }

kwalrath · 2020-08-13T20:08:01Z

src/_guides/language/effective-dart/usage.md

@@ -1072,8 +1072,10 @@ If a field doesn't depend on any constructor parameters, it can and should be
 initialized at its declaration. It takes less code and makes sure you won't
 forget to initialize it if the class has multiple constructors.

+<!-- Q: As of 2.9, this code no longer works.


@munificent opinion?

I mentioned this elsewhere, but to close the loop here, I think we should make contents non-final (assuming I'm guessing correctly at what no longer works).

kwalrath · 2020-08-13T20:09:49Z

src/_guides/language/sound-problems.md

 {% prettify dart tag=pre+code %}
 bool b = [0][0];
 {% endprettify %}

 With type-safe Dart, the analyzer produces the following error:

 {:.console-output}
-<?code-excerpt "strong/analyzer-results-stable.txt" retain="/'int' can't be .* 'bool'.*common_problems/"?>
+<!-- code-excerpt "strong/analyzer-results-stable.txt" retain="/'int' can't be .* 'bool'.*common_problems/" -->


Can we uncomment the code-excerpt tag? Otherwise, the analyzer message could change and we wouldn't know.

The problem is that since we removed the files that generate those errors, we can't get that error excerpt from those files (same applies to the other comments about this)

Well drat. I think it's fine for now, but since analysis messages do change, I could file a bug about fixing that.

Or... since analyzer messages are now listed & explained in https://dart.dev/tools/diagnostic-messages, for some messages at least we could have static (not included) text that links to the message description there (in another PR). An advantage of this approach is that if/when the message changes, @bwilkerson has started adding forwarding links to the old links. (See the discussion at #2569.)

kwalrath · 2020-08-13T20:10:10Z

src/_guides/language/sound-problems.md

@@ -55,9 +54,9 @@ see [Runtime errors](#common-errors-and-warnings).

 ### Undefined member

-<?code-excerpt "strong/analyzer-results-stable.txt" retain="/isn't defined for the type.*common_problems/" replace="/getter/<member\x3E/g; /'\w+'/'...'/g"?>
+<!-- code-excerpt "strong/analyzer-results-stable.txt" retain="/isn't defined for the type.*common_problems/" replace="/getter/<member\x3E/g; /'\w+'/'...'/g" -->


Can we uncomment the code-excerpt? (I have the same question for every other commented-out code-excerpt in this file where we show the error text. :)

src/_guides/language/sound-problems.md

kwalrath · 2020-08-13T20:13:25Z

tool/analyze-and-test-examples.sh

@@ -73,24 +73,6 @@ function analyze_and_test() {
    EXPECTED_FILE=$PROJECT_ROOT/analyzer-results.txt
  fi
  travis_fold start analyzeAndTest.analyze
-  if [[ -e $EXPECTED_FILE && -z $QUICK ]]; then


@johnpryan fyi

theacodes · 2020-08-14T14:49:17Z

let's file a bug for adding links for all analysis messages.

…

On Thu, Aug 13, 2020 at 5:49 PM Kathy Walrath ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/_guides/language/sound-problems.md <#2575 (comment)>: > {% prettify dart tag=pre+code %} bool b = [0][0]; {% endprettify %} With type-safe Dart, the analyzer produces the following error: {:.console-output} -<?code-excerpt "strong/analyzer-results-stable.txt" retain="/'int' can't be .* 'bool'.*common_problems/"?> + Well drat. I think it's fine for now, but since analysis messages do change, I could file a bug about fixing that. Or... since analyzer messages are now listed & explained in https://dart.dev/tools/diagnostic-messages, for some messages at least we could have static (not included) text that links to the message description there (in another PR). An advantage of this approach is that if/when the message changes, @bwilkerson <https://github.com/bwilkerson> has started adding forwarding links to the old links. (See the discussion at #2569 <#2569>.) — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#2575 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAB5I42TVO35ZXWGRF7YZDDSARNU7ANCNFSM4P5T7EEQ> .

kwalrath · 2020-08-14T21:56:56Z

@johnpryan could you approve this? Turns out I shouldn't because it started out as my PR...

johnpryan · 2020-08-14T22:19:25Z

Sure, taking a look

johnpryan

LGTM. I think we need to adjust our strategy for analyzing "bad" snippets, if we choose to try to analyze them again. It might be reasonable to keep them unanalyzed since we're still analyzing the "good" ones. The downside is that we will be relying on contributors checking the bad snippet if they make a change to the good one.

johnpryan · 2020-08-14T23:15:59Z

examples/misc/bin/try_dart/functions.dart

@@ -7,9 +7,9 @@ int timesTwo(int x) {
 int timesFour(int x) => timesTwo(timesTwo(x));

 // Functions are objects.
-int runTwice(int x, Function f) {
+int runTwice(int x, int Function(int) f) {


just FYI I just merged a change that moves this snippet into dartpad_picker_main. I created an issue to track: #2581

kwalrath added 8 commits August 12, 2020 18:35

Add function type to homepage DartPad

85f38a1

Effective Dart bad example no longer passes analysis

233950a

Another Effective Dart example that no longer passes

2e92ee5

Unexcerpt sound-problems code that fails analysis

18e4697

Unexcerpt language tour generics code that fails analysis

2d558ec

Remove unnecessary release names in ignore comment

2d0d973

Unexcerpt type-system code that fails analysis

3ece334

Current results: examples/misc

7833912

I'm not sure if any of these are used in the docs... but should we get the same results?

kwalrath assigned theacodes Aug 13, 2020

googlebot added the cla: yes Contributor has signed the Contributor License Agreement label Aug 13, 2020

kwalrath marked this pull request as draft August 13, 2020 15:57

kwalrath and others added 2 commits August 13, 2020 09:03

Don't let stable analysis fail

f3738c6

Fix expected analyzer results for examples/strong

0525765

This seemed to be broken becuase of a deleted/renamed file (`animal_bad.dart` & `common_problems_analysis.dart`).

kwalrath mentioned this pull request Aug 13, 2020

undefined_identifier and other errors are no longer ignorable, causing issues for generated parts dart-lang/sdk#42977

Closed

theacodes added 7 commits August 13, 2020 13:44

Update expected analysis output for library_tour.

a64ab42

These are existing warnings that we'd ignored that became unsilenceable errors in 2.9

Update expected analysis results for examples/analysis

0a88a2a

Fix missing line in misc/analysis-results.txt

c3355b7

Remove (seemingly?) redundant code from analyze-and-test-example.sh t…

66a2ace

…hat incorrectly ignores expected analysis-results files on travis

Bake in analysis errors for sound-problems.md, as the code we extract…

e6c098a

…ed these from has been removed due to it being invalid code in 2.9

Update excerpt tag in analyzer-options.txt to only grab the relevant …

0e555d1

…line

Update analyzer-results-dev.txt to match analyzer-results-stable.txt …

ef7995c

…for examples/strong

theacodes marked this pull request as ready for review August 13, 2020 19:52

theacodes changed the title ~~WIP: Fix sample analysis~~ Fix sample analysis build jobs Aug 13, 2020

kwalrath commented Aug 13, 2020

View reviewed changes

kwalrath mentioned this pull request Aug 14, 2020

Test homepage DartPad code #2578

Closed

Fix minor typo

a850e36

kwalrath requested a review from johnpryan August 14, 2020 21:56

kwalrath mentioned this pull request Aug 14, 2020

Add links from static analysis messages to diagnostic-messages? #2579

Open

kwalrath mentioned this pull request Aug 14, 2020

Change Effective Dart when "bad" code doesn't pass analysis #2580

Closed

johnpryan approved these changes Aug 14, 2020

View reviewed changes

kwalrath merged commit e0ec65f into master Aug 14, 2020

kwalrath deleted the kw-fix-build branch August 14, 2020 23:34

kwalrath mentioned this pull request Aug 19, 2020

Fix the beta build #2525

Closed

kwalrath mentioned this pull request Sep 2, 2020

Fix commented-out error messages in "Fixing common type problems" #2618

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix sample analysis build jobs #2575

Fix sample analysis build jobs #2575

kwalrath commented Aug 13, 2020 •

edited

Loading

kwalrath commented Aug 13, 2020

kwalrath commented Aug 13, 2020

theacodes commented Aug 13, 2020

kwalrath left a comment

kwalrath Aug 13, 2020

theacodes Aug 13, 2020

kwalrath Aug 13, 2020

theacodes Aug 13, 2020

kwalrath Aug 13, 2020

munificent Aug 25, 2020

kwalrath Aug 26, 2020

kwalrath Aug 13, 2020

munificent Aug 25, 2020

kwalrath Aug 13, 2020

theacodes Aug 13, 2020

kwalrath Aug 13, 2020

kwalrath Aug 13, 2020

kwalrath Aug 13, 2020

theacodes commented Aug 14, 2020 via email

kwalrath commented Aug 14, 2020

johnpryan commented Aug 14, 2020

johnpryan left a comment •

edited

Loading

johnpryan Aug 14, 2020

		@@ -1,18 +1,11 @@
		Analyzing analysis_options.yaml, lib, test...
		error • 'HoneyBadger.parent' ('Root Function()') isn't a valid override of 'Animal.parent' ('Animal Function()'). • lib/animal_bad.dart:15:12 • invalid_override

Fix sample analysis build jobs #2575

Fix sample analysis build jobs #2575

Conversation

kwalrath commented Aug 13, 2020 • edited Loading

kwalrath commented Aug 13, 2020

kwalrath commented Aug 13, 2020

theacodes commented Aug 13, 2020

kwalrath left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theacodes commented Aug 14, 2020 via email

kwalrath commented Aug 14, 2020

johnpryan commented Aug 14, 2020

johnpryan left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwalrath commented Aug 13, 2020 •

edited

Loading

johnpryan left a comment •

edited

Loading