F# Error When Adding suggested open is added on top of namespace and this breakes compilation

[vsfeedback]

This issue has been moved from a ticket on Developer Community.

---

[severity:It's more difficult to complete my work]
The suggestion for adding the open breakes the compilation as open is added on top, before naspace declaration.

Original Comments

Feedback Bot on 6/9/2021, 07:43 PM:

We have directed your feedback to the appropriate engineering team for further evaluation. The team will review the feedback and notify you about the next steps.

---

Original Solutions

(no solutions)

[Andreas-Dorfer]

To me it's not a big issue but it is annoying.

[vzarytovskii]

This only reproduces when namespace in on the 1st line of the document, because of this (ctx.Pos.Line = 1 here):

fsharp/src/fsharp/service/ServiceParsedInputOps.fs

Lines 1665 to 1677 in 76e65ac

	| ScopeKind.Namespace ->
	// for namespaces the start line is start line of the first nested entity
	if ctx.Pos.Line > 1 then
	[0..ctx.Pos.Line - 1]
	|> List.mapi (fun i line -> i, getLineStr line)
	|> List.tryPick (fun (i, lineStr) ->
	if lineStr.StartsWithOrdinal("namespace") then Some i
	else None)
	|> function
	// move to the next line below "namespace" and convert it to F# 1-based line number
	| Some line -> line + 2
	| None -> ctx.Pos.Line
	else 1

I'm not entirely sure why this is the logic, maybe @cartermp remembers?

[cartermp]

I didn't write that code so I'm not sure. I know that it was a bit tricky to debug though. Shouldn't the code also try to insert an open where other opens already are? So isn't the case here that it's (a) a namespace decl up top, and (2) no open decls as well?

If so I also think this should really be bumped down in priority, and it's also not a regression

[vzarytovskii]

I didn't write that code so I'm not sure. I know that it was a bit tricky to debug though. Shouldn't the code also try to insert an open where other opens already are? So isn't the case here that it's (a) a namespace decl up top, and (2) no open decls as well?

If so I also think this should really be bumped down in priority, and it's also not a regression

It first tries to determine where to insert the open, for that it either
a. If it's not the 1st line, then it will try to find the namespace decl in the 0..currentLine and return the line for open after it.
b. if it's 1st line it will just return 1 (which I think is incorrect).

I have added the check if we are on the first line - if it's the namespace line, return next one, otherwise, return 1 (as it was before).

[baronfel]

Aha! I think this caused some regressions for opens in FSAC as well during our FCS 41 update! We also try to do the things that @cartermp mentioned, to better or worse success.

[vzarytovskii]

It does try to align with existing opens, later on, after it calculates the line to insert to first time.

[vzarytovskii]

This is my version of the fix: https://github.com/dotnet/fsharp/pull/12443/files. Probably could be simplified with removing the codnition alltogether.
I've tested it on:

• namespace on 1st line
• namespace is not on 1st line
• with existing opens
• without opens
• without namespace.