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

Analyzer does not warn on function calls on Object #31509

Closed
stereotype441 opened this issue Dec 1, 2017 · 8 comments
Closed

Analyzer does not warn on function calls on Object #31509

stereotype441 opened this issue Dec 1, 2017 · 8 comments
Assignees
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. language-strong-mode-polish P1 A high priority bug; for example, a single project is unusable or has many test failures type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@stereotype441
Copy link
Member

This is the analyzer version of #21938.

For the analyzer, the change should be fairly straightforward: we just need to hardcode the enableStrictCallChecks option to true.

@stereotype441 stereotype441 added analyzer-strong-mode area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P1 A high priority bug; for example, a single project is unusable or has many test failures language-strong-mode-polish type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Dec 1, 2017
@bwilkerson
Copy link
Member

Does enableStrictCallChecks have other effects that shouldn't be enabled? Some of the comments (#21938 (comment) and #21938 (comment)) lead me to think that this would also flag the use of call on functions, and that this isn't a desired state.

@stereotype441
Copy link
Member Author

@bwilkerson Taking a quick glance through the code, I believe you may be right. So I guess this task may be a little more involved than just switching on the flag.

@mit-mit
Copy link
Member

mit-mit commented Dec 5, 2017

I believe the desired end-state is to remove the enableStrictCallChecks flag entirely, and to replace it with this behaviour: #21938 (comment). I asked language folks to confirm.

@mit-mit
Copy link
Member

mit-mit commented Dec 9, 2017

The good language folks confirmed that this is the expected behavior:

void test(dynamic d, Object o, Function f) {
  d(); //# 01: ok
  o(); //# 02: compile-time error
  f(); //# 03: ok
  d.call; //# 04: ok
  o.call; //# 05: compile-time error
  f.call; //# 06: ok
  d.call(); //# 07: ok
  o.call(); //# 08: compile-time error
  f.call(); //# 09: ok
}

@mit-mit
Copy link
Member

mit-mit commented Jan 11, 2018

Hi @bwilkerson, any update on this issue?

@stereotype441
Copy link
Member Author

@mit-mit looking at tests/language_2/language_2_analyzer.status, it looks like the analyzer currently fails test case 2 (it allows o(); and shouldn't - the test in question is called object_has_no_call_method_test). I've added this to #31638 to make sure it isn't forgotton. Hopefully either @scheglov or I can get to it soon.

@stereotype441
Copy link
Member Author

I'm working on this.

stereotype441 added a commit to stereotype441/mustache that referenced this issue Feb 13, 2018
In Dart 2.0, it will become a compile-time error to try to perform a
function invocation on a variable whose static type is `Object` (see
dart-lang/sdk#31509).  Since `value` is
changed inside the `if` block, it is not type promoted, so its type is
considered to be `Object`.  Therefore, to avoid a compile-time error,
we need to use an intermediate variable of type Function.
dart-bot pushed a commit that referenced this issue Feb 13, 2018
This reverts commit 6837daf.

Reason for revert: Broke analyzer bots.

Original change's description:
> Implement proper checking for callability of Function class.
> 
> There was some old (incorrect) logic for doing this, behind the flag
> enableStrictCallChecks.  This flag has been removed, since the new
> behavior is now standard in Dart 2.0.
> 
> Fixes #31509
> 
> Change-Id: I4a6da34a4b85ea8409f6e0d14c377a586546056a
> Reviewed-on: https://dart-review.googlesource.com/40509
> Commit-Queue: Paul Berry <paulberry@google.com>
> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
> Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
> Reviewed-by: Mike Fairhurst <mfairhurst@google.com>

TBR=paulberry@google.com,scheglov@google.com,brianwilkerson@google.com,mfairhurst@google.com

Change-Id: Ib631ad16bc5e937ff914127d1c5330f3fcaff2c9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/40980
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
@stereotype441
Copy link
Member Author

Reopening issue since the fix was reverted

@stereotype441 stereotype441 reopened this Feb 13, 2018
xxgreg pushed a commit to xxgreg/mustache that referenced this issue Jun 3, 2018
In Dart 2.0, it will become a compile-time error to try to perform a
function invocation on a variable whose static type is `Object` (see
dart-lang/sdk#31509).  Since `value` is
changed inside the `if` block, it is not type promoted, so its type is
considered to be `Object`.  Therefore, to avoid a compile-time error,
we need to use an intermediate variable of type Function.
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. language-strong-mode-polish P1 A high priority bug; for example, a single project is unusable or has many test failures type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

3 participants