Skip to content

Run the F# analyzers over every project and script - #249

Merged
nojaf merged 3 commits into
fsprojects:masterfrom
nojaf:analyzers
Sep 15, 2026
Merged

nojaf merged 3 commits into
fsprojects:masterfrom
nojaf:analyzers

Conversation

@nojaf

@nojaf nojaf commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Add an Analyze pipeline to build.fsx that runs fsharp-analyzers with
Ionide.Analyzers and G-Research.FSharp.Analyzers over the nine solution
projects, build.fsx and OldFsYaccTests.fsx in a single invocation, writing
one analysis.sarif for the repository. Generated lexer and parser sources,
AssemblyInfo.fs and the test SDK entry point are excluded, since findings
there are not fixable here.

The analyzer packages live in their own paket group with package storage,
so no project references them and the script finds them at a fixed path
without knowing the version.

Pull requests get a separate ubuntu-only analyze job, and pushes to master
run the same steps so code scanning has a baseline to diff against. Both
upload the report to GitHub code scanning and do not fail the workflow.

Remove tests/fsyacc/oldfsyacctests.fsx.lock, a leftover from when that
script pulled FAKE in through paket.

Add an Analyze pipeline to build.fsx that runs fsharp-analyzers with
Ionide.Analyzers and G-Research.FSharp.Analyzers over the nine solution
projects, build.fsx and OldFsYaccTests.fsx in a single invocation, writing
one analysis.sarif for the repository. Generated lexer and parser sources,
AssemblyInfo.fs and the test SDK entry point are excluded, since findings
there are not fixable here.

The analyzer packages live in their own paket group with package storage,
so no project references them and the script finds them at a fixed path
without knowing the version.

Pull requests get a separate ubuntu-only analyze job, and pushes to master
run the same steps so code scanning has a baseline to diff against. Both
upload the report to GitHub code scanning and do not fail the workflow.

Remove tests/fsyacc/oldfsyacctests.fsx.lock, a leftover from when that
script pulled FAKE in through paket.
The first analyzer run reported 153 findings, all warnings or lower. This
resolves them so the code scanning baseline starts clean and a new finding
on a pull request stands out.

Most of it is mechanical: postfix generic syntax throughout the runtime and
the tools, printf-style calls without format specifiers replaced by plain
writes, ordinal string comparisons made explicit, list functions instead of
Seq over lists, and a type argument on each bare `string` call.

A few changes touch the shape of the code. The Rule, Production and
ExplicitPrec union cases have named fields. Action is qualified access only,
because its Error case shadowed Result.Error. Associativity and Domain are
struct unions. TryDecodeUnicodeCategory returns a value option, so the
UnicodeCategoryAP active pattern can be struct-returning without a
conversion. The JSON example matches on the parse result rather than
reading .Value.

No behaviour changes. The fsyacc console hint about precedences reads the
same, its %% escapes just became single % now that it is no longer a
format string.
@github-advanced-security

Copy link
Copy Markdown

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

The pack step hands RELEASE_NOTES.md to MSBuild as a /p:PackageReleaseNotes
switch. MSBuild reads a newline, a semicolon or a comma in a property value
as the start of the next switch, so the multi-line notes were cut into
fragments and the Release pipeline failed with MSB1006 "Property is not
valid" on every push to master since the move to Fun.Build.

Each of those characters, and the others MSBuild gives meaning to, is now
written as its %XX escape. MSBuild unescapes the value when it reads the
property, so the nuspec carries the notes exactly as written.
@nojaf
nojaf merged commit 05e67d0 into fsprojects:master Sep 15, 2026
5 checks passed
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 this pull request may close these issues.

2 participants