diff --git a/src/Compiler/Service/ServiceAnalysis.fs b/src/Compiler/Service/ServiceAnalysis.fs index 38af8f7156..fa728886d1 100644 --- a/src/Compiler/Service/ServiceAnalysis.fs +++ b/src/Compiler/Service/ServiceAnalysis.fs @@ -451,3 +451,74 @@ module UnusedDeclarations = let unusedRanges = getUnusedDeclarationRanges allSymbolUsesInFile isScriptFile return unusedRanges } + +module public Naming = + + type UnnamedFieldsLocations = + | InDiscriminatedUnionCases + | InExceptions + + let defaultOptions = Set.ofArray [|InDiscriminatedUnionCases; InExceptions|] + + let fieldNotOk (field: FSharpField) = + field.IsNameGenerated + || isNull field.Name + || System.String.Empty = field.Name + + [] + let rec checkDeclaration (options: Set) declaration = + match declaration with + | FSharpImplementationFileDeclaration.MemberOrFunctionOrValue _ -> Seq.empty + | FSharpImplementationFileDeclaration.InitAction _ -> Seq.empty + | FSharpImplementationFileDeclaration.Entity(entity, _declarations) -> + seq { + if options.Contains InDiscriminatedUnionCases then + for case in entity.UnionCases do + // lenient on non public members + if not case.Accessibility.IsPublic then () else + // lenient on single field cases + if case.Fields.Count <= 1 then () else + for field in case.Fields do + if fieldNotOk field then + field.DeclarationLocation + match field.ImplementationLocation with + | None -> () + | Some range -> range + match field.SignatureLocation with + | None -> () + | Some range -> range + if options.Contains InExceptions then + if entity.IsFSharpExceptionDeclaration then + // lenient on non public members + if not entity.Accessibility.IsPublic then () else + // lenient on single field cases + if entity.FSharpFields.Count <= 1 then () else + for field in entity.FSharpFields do + if fieldNotOk field then + field.DeclarationLocation + match field.ImplementationLocation with + | None -> () + | Some range -> range + match field.SignatureLocation with + | None -> () + | Some range -> range + for declaration in _declarations do + yield! checkDeclaration options declaration + } + + let getUnnamedDiscriminatedUnionAndExceptionFields (checkFileResults: FSharpCheckFileResults, isScriptFile: bool) = + async { + let options = defaultOptions + // be lenient in case of script + if isScriptFile then return Seq.empty else + match checkFileResults.ImplementationFile with + | None -> return Seq.empty + | Some checkResults -> + return (seq { + for d in checkResults.Declarations do + yield! checkDeclaration options d + } + |> Seq.distinct // not comparable but distinct works :) + //|> Seq.sort // not comparable + ) + } diff --git a/src/Compiler/Service/ServiceAnalysis.fsi b/src/Compiler/Service/ServiceAnalysis.fsi index 672cf08875..d20b8ab0f7 100644 --- a/src/Compiler/Service/ServiceAnalysis.fsi +++ b/src/Compiler/Service/ServiceAnalysis.fsi @@ -31,3 +31,7 @@ module public UnusedDeclarations = /// Get all unused declarations in a file val getUnusedDeclarations: checkFileResults: FSharpCheckFileResults * isScriptFile: bool -> Async> + + +module public Naming = + val getUnnamedDiscriminatedUnionAndExceptionFields : checkFileResults: FSharpCheckFileResults * isScriptFile : bool -> Async> \ No newline at end of file diff --git a/tests/service/ProjectAnalysisTests.fs b/tests/service/ProjectAnalysisTests.fs index f7cb93317c..05c5e74be2 100644 --- a/tests/service/ProjectAnalysisTests.fs +++ b/tests/service/ProjectAnalysisTests.fs @@ -5765,3 +5765,41 @@ let ``References from #r nuget are included in script project options`` () = |> Seq.distinct printfn "%s" (assemblyNames |> String.concat "\n") assemblyNames |> should contain "Dapper.dll" + + +[] +let ``detects unnamed fields in DU and exceptions`` () = + let file = __SOURCE_DIRECTORY__ + "/data/analysis/unammed-fields/unnamed.members.diagnostics.fsx" + let baseline = System.IO.Path.ChangeExtension(file, ".bsl") + let input = + file + |> System.IO.File.ReadAllText + |> SourceText.ofString + let checker = FSharpChecker.Create(keepAssemblyContents=true) + + let projOptions, diagnostics = + checker.GetProjectOptionsFromScript(file, input) + |> Async.RunSynchronously + + let parseResults, checkFileAnswer = + checker.ParseAndCheckFileInProject(file, 0, input, projOptions) + |> Async.RunSynchronously + + match checkFileAnswer with + | FSharpCheckFileAnswer.Aborted -> failwith $"unexpected check file results for {file}" + | FSharpCheckFileAnswer.Succeeded results -> + let warns = + Naming.getUnnamedDiscriminatedUnionAndExceptionFields(results, false) + |> Async.RunSynchronously + |> Seq.toArray + |> Seq.map string + |> String.concat "\n" + let expected = System.IO.File.ReadAllText baseline + if expected <> warns then + let postmortem = System.IO.Path.ChangeExtension(file, ".err") + System.IO.File.WriteAllText(postmortem, warns) + let message = + $"expected:\n{expected}\ngot:\n{warns}\n" + + $"please compare\n{baseline}\nwith\n{postmortem}" + failwith message + \ No newline at end of file diff --git a/tests/service/data/analysis/unammed-fields/unnamed.members.diagnostics.bsl b/tests/service/data/analysis/unammed-fields/unnamed.members.diagnostics.bsl new file mode 100644 index 0000000000..ae5471e5ee --- /dev/null +++ b/tests/service/data/analysis/unammed-fields/unnamed.members.diagnostics.bsl @@ -0,0 +1,5 @@ +(4,19--4,22) +(10,13--10,16) +(14,26--14,29) +(20,20--20,23) +(24,33--24,36) \ No newline at end of file diff --git a/tests/service/data/analysis/unammed-fields/unnamed.members.diagnostics.fsx b/tests/service/data/analysis/unammed-fields/unnamed.members.diagnostics.fsx new file mode 100644 index 0000000000..35b685cdbb --- /dev/null +++ b/tests/service/data/analysis/unammed-fields/unnamed.members.diagnostics.fsx @@ -0,0 +1,27 @@ +type TopLevelDU = +| TopLevelCase1 of int +| TopLevelCase2 of ok: int * okok: string +| TopLevelCase3 of int * noOkCase: string + +module Outer = + type DU = + | Case1 of int + | Case2 of ok: int * okok: string + | Case3 of int * noOkCase: string + + exception Exception1 of int + exception Exception2 of ok: int * okok: string + exception Exception3 of int * noOkCase: string + + module Inner = + type InnerDU = + | InnerCase1 of int + | InnerCase2 of ok: int * okok: string + | InnerCase3 of int * noOkCase: string + + exception InnerException1 of int + exception InnerException2 of ok: int * okok: string + exception InnerException3 of int * noOkCase: string + + type PrivateDU = private PrivateCase1 of int * string + exception private PrivateException of int * string \ No newline at end of file