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

Improve error reporting: warning if returning ignore #1113

Closed
smoothdeveloper opened this issue Apr 25, 2016 · 8 comments
Closed

Improve error reporting: warning if returning ignore #1113

smoothdeveloper opened this issue Apr 25, 2016 · 8 comments

Comments

@smoothdeveloper
Copy link
Contributor

smoothdeveloper commented Apr 25, 2016

When beginning with F# (or when exhausted), it can happen to be writing code which mistakenly returns ignore such as:

[<Test>] let ``my test`` () =
  // some code

  ignore

I think this is in many case unintended and warrants a warning.

Did you intend to return 'ignore' or to use it to discard previous expression?
#1103

@forki
Copy link
Contributor

forki commented Apr 25, 2016

What's wrong with it? The fact that we don't ignore anything? And what is
the expected error message?
On Apr 25, 2016 4:59 PM, "Gauthier Segay" notifications@github.com wrote:

When beginning with F# (or when exhausted), it can happen to be writing
code which mistakenly returns ignore such as:

[] let my test () =
// some code

ignore

I think this is in many case unintended and warrants a warning.


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#1113

@smoothdeveloper
Copy link
Contributor Author

@forki, added the error message.

What is wrong is that the function signature in this case turns into unit -> ('a -> unit) instead of unit -> unit.

Try to put a NUnit test such as this one and to run it, I think I've seen this mistake few times when someone is not used to the pipe operator.

Of course if people think it is a stupid warning we can forget about it 😄

@isaacabraham
Copy link
Contributor

so in other words you meant to return () or just ignore()?

I guess this is the same as returning a function in any function - is what we're saying is "should we return a warning any time you return a function"? Can't be. Is ignore a special case though? Would you ever normally want to return it? Maybe not.

@smoothdeveloper
Copy link
Contributor Author

@isaacabraham intent is to just make that happen for ignore from FSharp.Core, it is OK to return a function otherwise (or to discard the warning if I really want to return ignore).

@smoothdeveloper smoothdeveloper changed the title Improved error reporting: warning if returning ignore Improve error reporting: warning if returning ignore Apr 25, 2016
@ReedCopsey
Copy link
Contributor

@smoothdeveloper I wonder if this would be better off as part of FSharpLint instead?

@forki
Copy link
Contributor

forki commented Apr 25, 2016

Yes this one looks too specific to "ignore" - I don't think we can bake that easily into the compiler. I mean there is probably a way, but it feels like this one should go into linter.

@forki
Copy link
Contributor

forki commented Apr 25, 2016

Or maybe it can go into nunit. The runner could complain if it finds tests with wrong signature

@smoothdeveloper
Copy link
Contributor Author

@ReedCopsey / @forki makes sense, I'll see if I can make a contribution to FSharpLint thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants