Open bartelink opened 6 months ago
nojaf
commented
6 months ago Hello,
While I don't disagree with the advice
This is interesting. I can't say that we based the analyzer on anything other than the feedback of the developers over at G-Research. Could you elaborate on why you disagree with this guidance? Your perspective would be insightful.
JsonSerializerOptions.Default seems like a good addition to mention indeed.
bartelink
commented
6 months ago I DONT disagree with the guidance, but, depending on how it's worded, it might prompt people to skip a potentially simpler solution.
My point is solely that the most common resolution for the literal example provided (creating default settings via the default ctor) already has an even better solution - use the in-the-box .Default property added in STJ V7.
The rest is just me saying "and if you have any Options builders that vary from STJ's Options.Default, then consider following that pattern in your impl:
.Default property alongside your builder method that caches a default-constructed set of options so the work can be cached cleanlynojaf
commented
6 months ago Whoops, I misread that. Thanks for elaborating.
While I don't disagree with the advice in https://g-research.github.io/fsharp-analyzers/analyzers/JsonSerializerOptionsAnalyzer.html#JsonSerializerOptionsAnalyzer, I'd add a mention of
JsonSerializerOptions.Default,Going further, I believe the better overall pattern is using a Serdes wrapper with a mandatory options arg a la https://github.com/jet/FsCodec/blob/master/src/FsCodec.SystemTextJson/Serdes.fs#L7, but I can appreciate that shoehorning that advice/guidance into an Analyzer might be a stretch!
(Aside: this can be taken even futher by having a lazily-implemented
.Defaultassociated with each suite ofOptionsa la https://github.com/jet/FsCodec/blob/master/src/FsCodec.SystemTextJson/Options.fs#L14 and following the general pattern outlined in https://github.com/jet/fscodec?tab=readme-ov-file#examples-of-using-serdes-to-define-a-contract )