Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dartc is too smart when dealing with variable declarations #3223

Closed
DartBot opened this issue May 25, 2012 · 11 comments
Closed

Dartc is too smart when dealing with variable declarations #3223

DartBot opened this issue May 25, 2012 · 11 comments
Assignees
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion.
Milestone

Comments

@DartBot
Copy link

DartBot commented May 25, 2012

This issue was originally filed by ms...@unipro.ru


The spes in the section 11.3 says: "A variable declaration statement var id; or var id = e; introduces a new variable named id with static type Dynamic into the innermost enclosing scope.".
This means there should not be any static warnings in this test:

func(int p) {}

main() {
  var param = true;
  func(param);
}

But Dartc, r7976, produces a static warning: "bool is not assignable to int".

Changing code to

func(int p) {}

main() {
  var param;
  param = true;
  func(param);
}

solves this problem.

@kasperl
Copy link

kasperl commented May 25, 2012

Adding Gilad on the CC list. I guess the issue here is an extra static type warning based on inferred types reported by the analyzer (trying to be helpful).


cc @gbracha.
Added Area-Analyzer, Triaged labels.

@bwilkerson
Copy link
Member

Set owner to @scheglov.

@peter-ahe-google
Copy link
Contributor

It is very tricky to warn based on inferred types.

It is essential that developers have a way to opt-out of warnings without losing code-completion. Also, the inferred type should not prevent the developer from completing members that aren't in the narrow inferred type.

While the intent is to help the user, it is not possible to infer why he didn't put a type on a variable.

A much simpler approach is to use inference to make suggestions about type annotations, and possible add automatically add the annotations in the source code while editing. How to do this well is tricky, and requires good UI.

Adding to that approach, it is possible to track two types of an expression. The inferred and the declared. This allows an analyzer to provide great information for code completion, but still obey the language semantics.

@gbracha
Copy link
Contributor

gbracha commented May 29, 2012

What Peter said. The current behavior is makes the spec meaningless. In general, we will need a much more nuanced interpretation of warnings in the tools. I would suggest that any warnings that go beyond the spec be packaged as a separate tool that can be run at the programmer's discretion, and displayed in a distinctive way.

@scheglov
Copy link
Contributor

@scheglov
Copy link
Contributor

@scheglov
Copy link
Contributor

We are going to re-introduce these checks again, but now it will be disabled for command line and can be enabled/disabled in Editor preferences.

http://code.google.com/p/dart/issues/detail?id=4518

@DartBot
Copy link
Author

DartBot commented Dec 19, 2012

This comment was originally written by rodion...@unipro.ru


command-line analyzer is still doing this:

  var varnotfunc = 1;
  try {
    varnotfunc();
    Expect.fail("NoSuchMethodError expected");
  } on NoSuchMethodError catch(ok) {}

WARNING|STATIC_TYPE|NOT_A_FUNCTION_TYPE|file:/media/sf_dart_local/co19/tests/co19/src/Language/11_Expressions/14_Function_Invocation/4_Function_Expression_Invocation_A02_t01.dart|55|5|10|"int" is not a function type

splitting variable declaration from initialization no longer works to suppress this warning either

@scheglov
Copy link
Contributor

Added this to the M3 milestone.
Added Accepted label.

@scheglov
Copy link
Contributor

@scheglov
Copy link
Contributor

@DartBot DartBot added Type-Defect area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. labels Dec 20, 2012
@DartBot DartBot added this to the M3 milestone Dec 20, 2012
copybara-service bot pushed a commit that referenced this issue Dec 1, 2021
Changes:
```
> git log --format="%C(auto) %h %s" b9edfa5..dcb6aba
 https://dart.googlesource.com/pub.git/+/dcb6abac Merge remote-tracking branch 'origin/cherry-pick2-for-2.15.0'
 https://dart.googlesource.com/pub.git/+/dc857523 Remove duplicated lines in testdata (#3234)
 https://dart.googlesource.com/pub.git/+/acc8ab09 Refactor the test package-server (#3230)
 https://dart.googlesource.com/pub.git/+/1e78c688 Better error messages round 2 (#3223)
 https://dart.googlesource.com/pub.git/+/eaf36513 More links in the repository specification (#3220)
 https://dart.googlesource.com/pub.git/+/c888b018 Remove pedantic and cleanup a few lints (#3224)
 https://dart.googlesource.com/pub.git/+/a4d44c7e Global package server null safety (#3225)
 https://dart.googlesource.com/pub.git/+/ff941887 Merge branch 'cherry-pick-for-2.15.0'
 https://dart.googlesource.com/pub.git/+/efd24e64 Fix hanging event handler for `stdin`. (#3218)
 https://dart.googlesource.com/pub.git/+/d77c14e8 Gitignore validator should not follow symlink dirs (#3209)
 https://dart.googlesource.com/pub.git/+/7c190789 migrate rest of test/ to null-safety (#3207)
 https://dart.googlesource.com/pub.git/+/f24adb64 migrate test/oath2/ test/outdated/ test/token test/validator/ to null-safety (#3206)
 https://dart.googlesource.com/pub.git/+/e7d77a57 migrate test/add, test/cache, test/embedding plus a bit more to null-safety (#3205)
 https://dart.googlesource.com/pub.git/+/a6a73ad2 migrate test/lish/, test/run/, test/upgrade/ and golden_file.dart to null-safety (#3204)
 https://dart.googlesource.com/pub.git/+/b90efc1f Add test that `pub get` works with no $HOME in environment. (#3173)
 https://dart.googlesource.com/pub.git/+/c2fe3966 migrate test/get/ to null-safety (#3203)
 https://dart.googlesource.com/pub.git/+/1d106898 migrate test/hosted/ to null-safety (#3200)
 https://dart.googlesource.com/pub.git/+/afa9932b migrate test/cache and test/downgrade/ to null-safety (#3201)
 https://dart.googlesource.com/pub.git/+/352ca989 migrate test/global/ to null-safety (#3202)
 https://dart.googlesource.com/pub.git/+/64f20ca4 migrate test/descriptor/ and couple more files in test/ to null-safety (#3192)

```

Diff: https://dart.googlesource.com/pub.git/+/b9edfa5e288ea3d1a57d1db054ef844ae7b27d99~..dcb6abac2d7d43258c03b348be42bf4aab9529b1/
Change-Id: I30d33b0b8c3c19097a16543db15c6ae633f0afc2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/221633
Auto-Submit: Jonas Jensen <jonasfj@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
Commit-Queue: Jonas Jensen <jonasfj@google.com>
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion.
Projects
None yet
Development

No branches or pull requests

6 participants