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: Detect module declarations that incorrectly end with = #1214

Closed
Tracked by #1103
DanTup opened this issue May 22, 2016 · 13 comments · Fixed by #1220
Closed
Tracked by #1103

Improve error reporting: Detect module declarations that incorrectly end with = #1214

DanTup opened this issue May 22, 2016 · 13 comments · Fixed by #1220

Comments

@DanTup
Copy link

DanTup commented May 22, 2016

Depending on whether you have a namespace or not, a module declaration may or may not require an =. This caught me out today (it's been a long time since I did any F#); I created a new file and deleted the namespace and got this compile error:

Files in libraries or multiple-file applications must begin with a namespace or module declaration

As I had some F# code open in my browser that had module blah = I copied it and continued to get the same message:

fsharp2

In hindsight, the example given in the message doesn't have an = so I should've figured it out, but it actually took much googling and finding a comment on this answer that made me realise the issue.

I think it might be worthwhile calling this out in the message (or ideally, detecting when there is a module declaration with an = that shouldn't be there and giving a specific message). It seems effort is going into improving messages in #1103.

@DanTup DanTup changed the title Provide more useful message relating to module declarations Improve error reporting: Detect module declarations than incorrectly end with = May 22, 2016
@isaacabraham
Copy link
Contributor

Good one. So I guess that this is caused by the = fooling the compiler into thinking that we want to use the filename as the module (and that NesTest is supposed to be a submodule)?

I'm not sure that the standard guidelines for messages that we've been following that @dsyme has mentioned will work in this case (i.e. "what is the error / why is it occurring / how can you resolve it") because of the nature of the error.

Assuming that it's possible to identify this situation, I'd suggest an error along the lines of: -

Only modules nested in namespaces or submodules require =. If this is a top-level module, consider removing the = to resolve this error.

@DanTup DanTup changed the title Improve error reporting: Detect module declarations than incorrectly end with = Improve error reporting: Detect module declarations that incorrectly end with = May 22, 2016
@dsyme
Copy link
Contributor

dsyme commented May 23, 2016

Suggest

Files in libraries or multiple-file applications must begin with a namespace or module declaration.  When using a module declaration at the start of a file, no '=' is required. If this is a top-level module, consider removing the = to resolve this error.

@isaacabraham
Copy link
Contributor

That's better, yes. Perhaps just just replace no '=' is required. to '=' is not required..

@forki
Copy link
Contributor

forki commented May 25, 2016

TBH I don't understand why we give an error in this case.

module X =
  let x = 1

Put this in a file called ModuleWithoutNamespace.fs and it doesn't compile. Why?

@forki
Copy link
Contributor

forki commented May 25, 2016

I mean it looks like a perfectly valid file. Copy the same code into fsi and it compiles. @dsyme why do we reject that.?

@isaacabraham
Copy link
Contributor

Isn't it because module X = indicates a submodule?

@forki
Copy link
Contributor

forki commented May 25, 2016

but why is that an issue?

If I disable the error message for a moment then it compiles just fine:

image

I mean why can't we improve the error message by not showing an error at all?

@forki
Copy link
Contributor

forki commented May 25, 2016

the code is like this:

if not (isLast && isExe) && not (Filename.checkSuffix filename ".fsx" )) then
errorR(Error(FSComp.SR.buildMultiFileRequiresNamespaceOrModule(),trimRangeToLine m))

So if we are in fsx file we just use the filename as "outer" module. Why can't we do that in fs files?

@isaacabraham
Copy link
Contributor

That only works for the last file in an exe AFAIK.... I've always wondered if that could be added for all files in a project.

@isaacabraham
Copy link
Contributor

I would suggest raising a separate issue for that though - it's not about error message but about changing compiler behaviour.

forki added a commit to forki/visualfsharp that referenced this issue May 25, 2016
@forki
Copy link
Contributor

forki commented May 25, 2016

WIP PR for error message #1220

@dsyme
Copy link
Contributor

dsyme commented May 26, 2016

Basically,

  • namespace N and module X are headers of an implementation file which says don't use the file name as the initial namespace/module, but use this name instead
  • module X = is a declaration of a nested module inside an enclosing module/namespace

We don't allow the second to be interpreted as the first because of the following

1 - In a script or last file, we allow programs like

let x = 1

In these cases, the code is places in a module taken from the file name, say FN.

2 - That implies that the following is valid code in a script or last file:

module X =
  let x = 1

module Y =
  let x = 1

This compiles to two nested types FN.X and FN.Y.

3 - So, in the last file, we also expect this to be valid code:

module X =
  let x = 1

Further the compilation representation should be predictable and consistent: deleting Y shouldn't change the compilation representation of X

4 - Adding a file shouldn't change the compilation representation of existing modules. Hence adding a file after this last file shouldn't change the compilation representation of X.

@KevinRansom
Copy link
Member

@dsyme
that is the best description of that behavior I have ever encountered. Perhaps it would be worthwhile adding a page to the visualfsharpdocs that express the module naming behavior this clearly.

forki added a commit to forki/visualfsharp that referenced this issue May 30, 2016
forki added a commit to forki/visualfsharp that referenced this issue May 30, 2016
forki added a commit to forki/visualfsharp that referenced this issue May 30, 2016
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

Successfully merging a pull request may close this issue.

5 participants