This is a great suggestion! I can see how this could be confusing. [Guesswork follows, possibly incorrect suggestion] Perhaps we could special-case the situation of "cannot assign value of type () to type T" (for any T); in such a case, maybe we can check if the issue is coming from a mutating method on the type T itself. If so, we can emit a special-case diagnostic.

@swift-ci create

[Note: In case you want to mark this as a StarterBug, let's have a discussion on the wording before marking it as such.]

Comment by Nikhil (JIRA)

theindigamer (JIRA User)

I would like to take the plunge myself to fix it as you would suggest (with the wording and all)

This is my first time, so please guide me with the starting steps.

Thanks.

You can start by installing Xcode 12 beta (if you are on macOS) or the appropriate dependencies (if you are on Linux) and building the project over the weekend. You can see the project README for detailed instructions.

Some other things to do in the meantime would be to:

1. Go through some of our external resources in the 'Contributing Guides and Tips' section (https://github.com/varungandhi-apple/swift/blob/vg-all-new-docs/docs/ExternalResources.md). I particularly recommend the Becoming an Effective Contributor to Swift talk. That talk also includes how to use the Xcode debugger for the Swift project.

2. Look at DiagnosticsSema.def and find the name of the "cannot assign value of type" diagnostic, and see where it is emitted in the source by searching for the name.

3. After step 2, you might have multiple places which emits this diagnostic. You can try to figure out which spot in the compiler is emitting the diagnostic for this particular example (by using the debugger or print debugging).

cc @hborla – can you suggest next steps? I can't tell how we could obtain the contextual information needed to emit this specialized diagnostic.

Comment by Nikhil (JIRA)

theindigamer (JIRA User)

Thank you. Will work on the steps provided.

Comment by Nikhil (JIRA)

I have been working on build, I have cloned master and did update checkout on my macOS 10.15.5.

EDIT: Build is successful.There is no scheme "swift" in the list, will check.

Also, I have narrowed down the code which is emitting our diagnostic in question to class ContextualFailure::diagnoseAsError() in CSDiagnostics.cpp.

Comment by Nikhil (JIRA)

I have debugged the issue and narrowed it down to

IgnoreAssignmentDestinationType::diagnose

Will try to guess something for

I can't tell how we could obtain the contextual information needed to emit this specialized diagnostic.

Yeah, the schemes have changed slightly around since that talk. There should be a swift-frontend scheme which you can use.

Comment by Nikhil (JIRA)

theindigamer (JIRA User) this is itching me for sometime now. I have tried various ways to narrow the “from type” to a mutating method like trying out (crude?) things like getting method parameters, checking for curriedself parameter etc.

I think the current diagnostic is okay. "Mutating functions don't return their types" is not correct, because a mutating function can return any type. For example, this is valid:

struct S {
  var value = 0
  mutating func foo() -> Self { // returns a value of type 'S'
    value = 1
    return self
  }
}

Although, maybe we can reword the diagnostic, something like 'append(:_)' returns a value of type '()', which cannot be assigned to variable 'string' (whose type is 'String'). On top of that, we could offer a fix-it, for example: remove assignment to 'string'.

It's a lot more verbose though, but perhaps more beginner friendly.

Anyhow, to answer your original question - a mutating function has a MutatingAttr on it. So, if you want to check if a function is mutating, then you first need to access the FuncDecl. Then, you can call isMutating() on it (which handles some special cases as well, apart from checking for MutatingAttr. I don't think you can infer this solely from the function type though.

I agree with the point in the Jira that the current diagnostic is not okay. Especially if you are coming from C, C++ or ObjC, where you are not used to void being inhabited (but Swift's Void has 1 inhabitant). Moreover, Swift allows you to omit Void from the return type, so it might look like nothing is being returned. In fact, we already have a warning today to account for this:

func f(_ s: inout String) {
    let newString = s.append("AAA"); // warning: constant 'newString' inferred to have type '()', which may be unexpected
}

This warning appears when the value being assigned to does not have a user-annotated type.

Given that, I think the special case to go for here would be: if the RHS has a call to a Void-returning mutating method and the LHS does not have a user-annotated type of Void, then we

1. emit a special case diagnostic, perhaps something like 'append(:_)' is a mutating method on 'String'; it does not return a new 'String'. This precisely addresses the user's mistake; it is likely that they were thinking that the method was nonmutating and returning a new String. The first 'String' comes from the type the method is on, the second 'String' comes from the type of the LHS.

2. emit a fix-it removing the assignment operation altogether. The fix-it would also fire in cases like the warning example above.

Hmm, okay. I was thinking more generally, but I think your suggested special-case diagnostic would be good to add! Should there be some special case fix-its too for common standard library mutating methods? For example, Array has sort and sorted and a fix-it could suggest changing sort to sorted.

I think it would be really nice to map standard library mutating methods to their functional/nonmutating variants to provide a second Fix-It to call the nonmutating one instead! However, I think that's outside the scope of a starter bug, so perhaps we should file a separate issue for that.

As for the new error message, I think this warrants two tailored messages. I think saying cannot assign value of type '()' to type 'String' is confusing for any Void-returning function. Also, I really like Varun's suggestion, but I think that should be a note rather than the error message because it doesn't explicitly state what the error is, but rather it provides more context on what led to the exact error. What about something like:

  str = str.appending("in Swift!")
      ^
error: cannot assign result of Void-returning [function/method] to type 'String'

  str = str.appending("in Swift!")
            ^
note: 'appending' is a mutating method on 'String'; it does not return a new 'String'

with the Fix-Its attached to the note?

Comment by Nikhil (JIRA)

I think @hborla 's approach covers all the three well:

1. Improved diagnostic (I believe) cos we are gonna replace `()` with `Void-returning [function/method]`,

2. Note giving more context,

3. Fix-it

Shall we lock this?

Yeah, I think the combination of tweaked error message + note would be nice! And we could create a new ticket to track adding fix-it to suggest changing from calling mutating to non-mutating variant of the functions...

hassaneldesouky (JIRA User) looks like this is available to work on!

Comment by Hassan ElDesouky (JIRA)

@hborla Thank you, I'll take a look at it.

Comment by Hassan ElDesouky (JIRA)

saidhon (JIRA User) You can take this if you want, I have a tight schedule so I haven't started in it, yet.