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

The argument type 'x' can't be assigned to the parameter type 'x' #32042

Closed
DanTup opened this Issue Feb 5, 2018 · 42 comments

Comments

Projects
None yet
10 participants
@DanTup
Copy link
Member

DanTup commented Feb 5, 2018

I think we've seen this before on Windows and believed it to be imports mixing 'package:' and relative paths, however I've just hit this today on a framework type:

file: 'file:///m%3A/Coding/Applications/flutter/packages/flutter_tools/lib/src/doctor.dart'
severity: 'Error'
message: 'The argument type 'Iterable<DoctorValidator> (M:\Apps\Dart\Dart-2.0.0-dev.20.0\lib\core\iterable.dart)' can't be assigned to the parameter type 'Iterable<DoctorValidator> (M:\Apps\Dart\Dart-2.0.0-dev.20.0\lib\core\iterable.dart)'.'
at: '47,28'
source: 'dart'
code: 'argument_type_not_assignable'

Note that the types are the same:

'Iterable<DoctorValidator> (M:\Apps\Dart\Dart-2.0.0-dev.20.0\lib\core\iterable.dart)'
'Iterable<DoctorValidator> (M:\Apps\Dart\Dart-2.0.0-dev.20.0\lib\core\iterable.dart)'

The source code is this revision:

DanTup/flutter@601655b

Originally the VsCodeValidator.installedValidators returned a List (not Iterable) but I got an equally confusing message (can't assign the list to an argument of iterable) so I change it to iterable while debugging and then it still complained.

I can try this on my Mac later, but I wouldn't be surprised if this was Windows-only given the previous similar error we had was.

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Feb 5, 2018

(@bwilkerson FWIW this doesn't seem to crash at runtime, so maybe just an analysis issue?)

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Feb 5, 2018

Actually, this is the same issue - I used the code-fix to add the import and it added it was a package: uri (I did look at this before I posted, but obviously not well enough!).

Changing import 'package:flutter_tools/src/vscode_validator.dart'; to import 'vscode_validator.dart'; fixes it.

I found the other issue - it's #28895.

@DanTup DanTup closed this Feb 5, 2018

@bwilkerson

This comment has been minimized.

Copy link
Member

bwilkerson commented Feb 5, 2018

Yep. In Dart, two libraries are the same if, and only if, they are imported using the same URI. If two different URIs are used, even if they resolve to the same file, they will be considered to be two different libraries and the types within the file will occur twice, leading to exactly this problem.

As a result, the recommendation used to be that you always use a package: URI to import code from within the lib directory, even when the importing library is within the lib directory. Doing so will avoid this problem. Partially as a result of that recommendation, analyzer is written to assume that every library within the lib directory was originally referenced using a package: URI (for the purposes of computing identity). Similarly, we intentionally made the quick fix insert a package: URI in order to make user's code consistent with this recommendation.

That said, the opposite approach also works. If all URIs within a package are always relative (and two packages are not mutually recursive), then you still avoid this problem. (If you have mutually dependent packages, then using package: URIs everywhere is the only sure solution.)

The key is to be consistent. Unfortunately, the tooling assumes the former scheme, and doesn't maintain consistency for the latter scheme.

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Feb 5, 2018

@bwilkerson If I understood correctly from the #28895, that issue only occurred on Windows and on Mac it was fine. I've been meaning to dig into it to see if I can debug, but haven't had chance yet (but I still have a reminder in my inbox).

@bwilkerson

This comment has been minimized.

Copy link
Member

bwilkerson commented Feb 5, 2018

I'll be interested to hear what you find.

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Feb 5, 2018

Me too! Is there a convenient place I can breakpoint to inspect an error like this to see what it's comparing (so I can see how it differs between Mac/Win)?

@bwilkerson

This comment has been minimized.

Copy link
Member

bwilkerson commented Feb 5, 2018

The way I usually tackle an issue like this is to figure out which error code defines the error you're seeing (should be defined in https://github.com/dart-lang/sdk/blob/master/pkg/analyzer/lib/src/error/codes.dart). Then find all references to the error code and put a breakpoint at the places where it's being used to create a diagnostic (there's often only one place). That usually gets me to the problematic code fairly quickly.

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Feb 5, 2018

@bwilkerson Great, thanks!

@maryx

This comment has been minimized.

Copy link

maryx commented Feb 12, 2018

As a result, the recommendation used to be that you always use a package: URI to import code from within the lib directory, even when the importing library is within the lib directory. Doing so will avoid this problem. Partially as a result of that recommendation, analyzer is written to assume that every library within the lib directory was originally referenced using a package: URI (for the purposes of computing identity). Similarly, we intentionally made the quick fix insert a package: URI in order to make user's code consistent with this recommendation.

@bwilkerson

  1. Is this still the recommendation? For Windows, or for general development?
  2. If it is, is this something we could mention in the Effective Dart style guide? https://www.dartlang.org/guides/language/effective-dart/style#do-place-package-imports-before-relative-imports implies that that relative imports are acceptable.

Thanks!

@zoechi

This comment has been minimized.

Copy link
Contributor

zoechi commented Feb 12, 2018

@maryx I never encountered this except in Flutter where main.dart is in lib/ instead of bin/ or web/ and it was mentioned that it's only important in lib/main.dart to not use relative imports.

If mixing import styles would cause issues in general, the only sane way of handling that would be to entirely remove relative imports from the language.

@lrhn

This comment has been minimized.

Copy link
Member

lrhn commented Feb 12, 2018

I think the rule I use is "never have lib in a path, and always use relative imports if possible".

It's not how I think about it, but it really boils down to that. Always use relative imports inside a package, always use package: URIs to refer to a package from outside of it. (It also prevents you from using relative paths from inside a package to outside of it because the package: URI doesn't allow that).

If you seem to need to say lib/ (where lib is the location of a package's files) to refer to a file, then you are not in the same package, and you should be using a package URI.

It also holds for entry-points. If you want to run dart mypackage/lib/main.dart, then do dart package:mypackage/main.dart. Never mention the lib!

That is where the actual error is in this case—there is a reference into a package using a non-package URI—and all the changing of URIs in the package are just trying to avoid the symptoms, not the root-cause of the error.

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Feb 12, 2018

I think the rule I use is "never have lib in a path, and always use relative imports if possible".

Always use relative imports inside a package

The analyzer currently returns a fix to use the package: version of the url ahead of the relative path:

Imports

Server response

If the guidance is to not do this then I think the analyzer should be updated to change the order (or even drop the package: import). This could be leading people towards mixing imports and ending up with errors like this.

@lrhn

This comment has been minimized.

Copy link
Member

lrhn commented Feb 12, 2018

The error is not due to the imports. It doesn't (well, shouldn't) matter whether a package library uses a relative or absolute path to refer to another library in the same package, it should always refer to the same library.
You only get a problem when it doesn't refer to the same library, and that only happens if the importing file is itself referenced incorrectly. So, the import is not the problem and "fixing" the problem by using an absolute import is actually hiding the problem. I'm lawful neutral and prefer to crash programs with problems sooner rather than later.

So, the error is to have lib in a library path or entry point path. Never do that, and then it doesn't matter what else you do inside a library. (Personally, I'd pick the relative import fix in this case, but again, it doesn't actually matter).

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Feb 12, 2018

If I'm understanding correctly, this seems different to what @bwilkerson said?

In Dart, two libraries are the same if, and only if, they are imported using the same URI. If two different URIs are used, even if they resolve to the same file, they will be considered to be two different libraries and the types within the file will occur twice, leading to exactly this problem.

That said, based on #28895 I think the behaviour for this may be different between Windows/Mac right now. I wanted to investigate #28895 but got stuck because of #32095 (I think they might be related, but one is user-fixable and the other is not).

Personally I'd prefer that the files are treated the same (even if imported with different uris) but whatever the decision I think it must be documented so we know what's a bug and what's not.

@lrhn

This comment has been minimized.

Copy link
Member

lrhn commented Feb 12, 2018

What @bwilkerson said is correct. A relative URI reference is not a URI. It's converted into a URI by resolving it relative to the library's own URI.
That's why it doesn't matter whether the library package:foo/foo.dart uses import "src/bar.dart"; or import "package:foo/src/bar.dart";, both will load the same library with URI package:foo/src/bar.dart.

If you somehow import file:///.../mypkgs/foo/lib/foo.dart, then you have made a mistake. The relative import will then propagate the mistake, and the absolute import will not, but the error has been made independently of that, and it should be fixed.

Windows has special issues because its file system is case indifferent and URIs are not, so importing src/bar.dart and src/Bar.dart will import the same file but will import different libraries (because they are different URIs). There is no good fix at the analyzer level except giving a warning so the user can fix it, because the language specification uses URIs to identify libraries. Trying to somehow make package:foo/src/bar.dart and package:foo/src/Bar.dart be considered the same library is, regrettably, a spec violation.

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Feb 12, 2018

That's why it doesn't matter whether the library package:foo/foo.dart uses import "src/bar.dart"; or import "package:foo/src/bar.dart";, both will load the same library with URI package:foo/src/bar.dart.

Ok, I think I understand you now. However, the original code that caused me to raise this is:

import 'package:flutter_tools/src/vscode_validator.dart';
import 'vscode_validator.dart'

The top one causes the error, the button one works. Are you saying these should be considered the same? (If so, I think this is a bug to re-open?)

@lrhn

This comment has been minimized.

Copy link
Member

lrhn commented Feb 12, 2018

The only way that those imports can do something different if in a file that is in flutter_tools/lib/src is that you are importing that file with a path that isn't package:flutter_tools/src/thatfile.dart. Somewhere you have a lib in a path. So, stop doing that instead of munging with the imports, it isn't their fault :).

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Feb 12, 2018

This is the flutter_tools package in the flutter repo; and you can repro without any of my changes (on Windows, at least). For example by changing (in `packages/flutter_tools/lib/src/doctor.dart):

import 'android/android_studio_validator.dart';

to

import 'package:flutter_tools/src/android/android_studio_validator.dart';

This causes the same error. I can't see an obvious issue with any of the imports there.

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Feb 12, 2018

These are all the imports for the doctor.dart file (which is the one that defines the type DoctorValidator which is the one in the error):

Doctor imports

I'm not saying there's nothing wrong in the code; but I don't know what it is but I'd feel better to know :)

@lrhn

This comment has been minimized.

Copy link
Member

lrhn commented Feb 12, 2018

How do you start the program? I'm guessing executable.dart is the entry point. If you start it using dart M:\Coding\Applications\flutter\packages\lib\executable.dart then that's the problem and you should do dart package:flutter_tools/executable.dart instead.

Normally a Dart app would put its entry point in flutter_tools/bin/ instead and then do normal package:-URI imports of the supporting package. That allows you to do pub global activate with the package to "install" it as an application.

In this case, you might want to consider executable.dart as morally external to the package, even if it lives in the lib dir (which is dirty, but probably close to the truth), and use absolute imports in that file only.
It is still a problem if executable.dart declares any public types, but since nobody else seems to be importing it, it's probably safe.

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Feb 12, 2018

You shouldn't need to run it, since it's the analysis that's reporting the error. But if it's useful, here's my launch config (the entry is in bin):

{
	"name": "Flutter Doctor",
	"program": "${workspaceRoot}/bin/flutter_tools.dart",
	"args": [
		"doctor",
		"-v"
	],
	"request": "launch",
	"type": "dart-cli"
},
@lrhn

This comment has been minimized.

Copy link
Member

lrhn commented Feb 12, 2018

Ah, makes sense. I think the analyzer needs to recognize lib dirs in package source and always consider their content as imported using package: URIs unless otherwise stated. So yes, I would blame the analyzer in this case, unless someone is importing package libraries with non-package: URIs.

The .../bin/flutter_tools.dart file looks fine. It uses a package: URI to reference the executable.dart file.

The fuchsia_builder.dart and fuchsia_tester.dart files, on the other hand, are full of ../lib/src/ imports which should be fixed. Maybe these are the source of the problem?

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Feb 12, 2018

Seems like I've opened a can of worms! I'm re-opening this since it seems like something is wrong somewhere, but I'm not sure where or who is best placed to look.

I did try changing all the imports in fuchsia_builder.dart and fuchsia_tester.dart to package:flutter_tools however it did not fix the issue (possibly because those files aren't imported by any of the others?), so possibly there's an analysis server issue.

Seems like those imports in the Fuchsia files have been touched be a lot of different people so I'm not sure who is best to ping about fixing them if they're deemed wrong?

@DanTup DanTup reopened this Feb 12, 2018

@bwilkerson

This comment has been minimized.

Copy link
Member

bwilkerson commented Feb 12, 2018

Is this still the recommendation? For Windows, or for general development?

This is still the recommendation that I give, but as you can see there are others that recommend using relative paths from libraries inside lib to other libraries in the same lib.

I think the analyzer needs to recognize lib dirs in package source and always consider their content as imported using package: URIs. So yes, I would blame the analyzer in this case.

That is, in fact, exactly what analyzer does (and has been doing for years).

My only guess, at this point, is that somehow on Windows we're ending up with two URIs that have different case somewhere, despite the fact that once they've been resolved to a file path the paths are identical.

@DanTup Is this a hypothesis you could investigate? One way to do that is to find the location(s) at which that error is being produced and either add a breakpoint or modify the code to use the URI rather than the file path when constructing the error message.

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Feb 12, 2018

Yep; this is one of the things I wanted to investigate (as mentioned above, I suspected it might be Windows-only) but hit the other Windows-analyzer issue. I think we might have a reasonable fix for that now though, so I should be able to debug further.

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Feb 12, 2018

(I don't currently have any access in this repo, feel free to assign it to me)

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Feb 13, 2018

This appears to be fixed by #32133 (normalisation of drive letters on windows, the fix for #32095).

@dart-bot dart-bot closed this in 56acaee Feb 13, 2018

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Mar 28, 2018

Re-opening this since the fix mentioned above was reverted and this might be a much easier case to address/fix than #32095 so it'd make sense to get out of the way first.

@DanTup DanTup reopened this Mar 28, 2018

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Mar 28, 2018

Whoops, #28895 was the case I intended to re-open. I think this one is highly likely a dupe of either #28895 or #32095.

@DanTup DanTup closed this Mar 28, 2018

@xorinzor

This comment has been minimized.

Copy link

xorinzor commented Jul 19, 2018

I had this issue too, and it took me a while to figure out why.

Turns out one file still had the import with the all-lowercase directory name "models", and another used the capitalized "Models".

I find it strange that android-studio still accepted the import as valid, even though the directory name wasn't all-lowercase anymore. Especially since whether it was capitalized or not, it still pointed to the same file if I ctrl+clicked on it.

@lrhn

This comment has been minimized.

Copy link
Member

lrhn commented Jul 19, 2018

@xorinzor Most likely, you are running on Windows.
The two URIs file:///C:/something/Models/something.dart and file:///C:/something/models/something.dart are different URIs (they have different content, so they are not the same).
When converted to a Windows file path, the two paths refer to the same file.

So, from everybody's perspective, you are referencing perfectly valid files using two different URIs, so you get two different libraries.

We should probably do something about this, it's too much of a foot-gun, and it's easily detectable that it really is the same Windows file.

@xorinzor

This comment has been minimized.

Copy link

xorinzor commented Jul 19, 2018

Yeah I'm using windows.

I can understand the issue with windows not caring about capitalization, where linux does.

But as you said yourself, this is really a kind of issue where it's just an absolute pain to debug the cause. So a fix would be really appreciated :)

Not sure how hard it'd be to implement linux-like behaviour, where it will just error on the import if the path (case-sensitive) doesn't match.

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Jul 19, 2018

A Windows user generally wouldn't expect an error because they wrote import 'server.dart' but on disk they called it Server.dart and in many cases, it'll work fine as long as they didn't mix and match, so it feels overly strict.

Normalising paths to their file-system equiv internally when resolving the path may be an option; but it could also cause weird issues if an editor sent a request to the analysis server cased one way but the response had a different casing - so I don't think it's as simple as converting everything one way on the way in.

My preferred option would be keep the case provided by the user/editor and fix all comparisons to do the right thing. There are a LOT of places this affects though - and I guess it's not just path comparisons, because it really affects the identity of types - you need to know that A from foo.dart and A from Foo.dart are the same if (and only if) the filesystem they map to is case insensitive (to make things worse case-sensitivity can be different across different drives on the same machine).

If it makes you feel better, this issue isn't isolated to Dart - there are so many npm packages that make assumptions about paths being case-sensitive - check how many issues reference this VS Code issue which as far as I can tell, nobody has any solid ideas for fixing. Uh, maybe that's not making anyone feel better.

(probably we should continue discussion in #28895 since that's the open issue for this problem, even though the title is a little misleading since it mentions only drive letters)

@worstkiller

This comment has been minimized.

Copy link

worstkiller commented Oct 9, 2018

Yep. In Dart, two libraries are the same if, and only if, they are imported using the same URI. If two different URIs are used, even if they resolve to the same file, they will be considered to be two different libraries and the types within the file will occur twice, leading to exactly this problem.

As a result, the recommendation used to be that you always use a package: URI to import code from within the lib directory, even when the importing library is within the lib directory. Doing so will avoid this problem. Partially as a result of that recommendation, analyzer is written to assume that every library within the lib directory was originally referenced using a package: URI (for the purposes of computing identity). Similarly, we intentionally made the quick fix insert a package: URI in order to make user's code consistent with this recommendation.

That said, the opposite approach also works. If all URIs within a package are always relative (and two packages are not mutually recursive), then you still avoid this problem. (If you have mutually dependent packages, then using package: URIs everywhere is the only sure solution.)

The key is to be consistent. Unfortunately, the tooling assumes the former scheme, and doesn't maintain consistency for the latter scheme.

For me this was the issue, different import for the same file, I was like mad from last 3-4 days and nothing seems to be working, then I read this. thanks now it's working.

@jerinho

This comment has been minimized.

Copy link

jerinho commented Dec 10, 2018

this too long conversation makes me dizzy. please anyone tell me which comment provide the solution? thank you. much appreciated

@zoechi

This comment has been minimized.

Copy link
Contributor

zoechi commented Dec 11, 2018

@jerinho
Please consider asking support questions in one of the other channels listed at http://flutter.io/support .

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Dec 11, 2018

@jerinho Most common causes of this are mixing package: and relative path imports or having casing differences in the paths.

@lukepighetti

This comment has been minimized.

Copy link

lukepighetti commented Dec 20, 2018

I'm running into this right now

The argument type 'int (/Users/xxx/flutter/bin/cache/pkg/sky_engine/lib/core/int.dart)' can't be assigned to the parameter type 'int (/Users/xxx/code/flutter/hydrate/example/lib/hydrated.dart)'.dart(argument_type_not_assignable) int val

I am trying to create a new class by extending Subject from rxdart within the example folder of a Flutter package.

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Jan 2, 2019

@lukepighetti Your error looks different:

int (/Users/xxx/flutter/bin/cache/pkg/sky_engine/lib/core/int.dart)
int (/Users/xxx/code/flutter/hydrate/example/lib/hydrated.dart)

This looks like you may have a class named int in the file code/flutter/hydrate/example/lib/hydrated.dart? Is that a public project we can see the source for?

@lukepighetti

This comment has been minimized.

Copy link

lukepighetti commented Jan 2, 2019

Hey @DanTup , I was able to get around this, but I honestly can't remember how. Forgot to report back here.

@DanTup

This comment has been minimized.

Copy link
Member Author

DanTup commented Jan 2, 2019

Glad you sorted it, but do shout if you see it again and can't figure it out :-)

@emuthomi

This comment has been minimized.

Copy link

emuthomi commented Jan 18, 2019

I have landed here after spending an hour or so fighting this:

Error: The argument type '#lib1::JobModel' can't be assigned to the parameter type '#lib2::JobModel'.
Try changing the type of the parameter, or casting the argument to '#lib2::JobModel'

type casting didn't help as suggested but after digging deeper it's when I saw where the error was:

I/flutter ( 1432): type 'JobModel' is not a subtype of type 'JobModel' where
I/flutter ( 1432):   JobModel is from package:myapp/models/JobModel.dart
I/flutter ( 1432):   JobModel is from package:myapp/models/jobModel.dart

Turned out to be an uppercase/lowercase typo in one of the imports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment