From 0f010afce10584d2b03bef3921162f6ce1461b36 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Tue, 6 Jun 2023 16:53:26 +0200 Subject: [PATCH 1/8] C#: Add dotnet test that targets dll. --- csharp/ql/integration-tests/posix-only/dotnet_test/test.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/csharp/ql/integration-tests/posix-only/dotnet_test/test.py b/csharp/ql/integration-tests/posix-only/dotnet_test/test.py index 7bc159e6720d..1012454003b5 100644 --- a/csharp/ql/integration-tests/posix-only/dotnet_test/test.py +++ b/csharp/ql/integration-tests/posix-only/dotnet_test/test.py @@ -1,5 +1,10 @@ from create_database_utils import * from diagnostics_test_utils import * -run_codeql_database_create(['dotnet test'], db=None, lang="csharp") +# Implicitly build and then run tests. +run_codeql_database_create(['dotnet test'], test_db="test-db", lang="csharp") check_diagnostics() + +# Explicitly build and then run tests. +run_codeql_database_create(['dotnet clean', 'rm -rf test-db', 'dotnet build -o myout', 'dotnet test myout/dotnet_test.dll'], test_db="test2-db", lang="csharp") +check_diagnostics(test_db="test2-db") \ No newline at end of file From 4dae7ad35aa7c7a040142e56d081c1e4216eacea Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Tue, 6 Jun 2023 17:22:52 +0200 Subject: [PATCH 2/8] C#: Only inject the shared compilation flag, if argument is not exe or dll. --- csharp/tools/tracing-config.lua | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/csharp/tools/tracing-config.lua b/csharp/tools/tracing-config.lua index 79b2ea2ca1c3..7fa03af2de13 100644 --- a/csharp/tools/tracing-config.lua +++ b/csharp/tools/tracing-config.lua @@ -23,6 +23,7 @@ function RegisterExtractorPack(id) local match = false local dotnetRunNeedsSeparator = false; local dotnetRunInjectionIndex = nil; + local libOrExe = false; local argv = compilerArguments.argv if OperatingSystem == 'windows' then -- let's hope that this split matches the escaping rules `dotnet` applies to command line arguments @@ -37,7 +38,7 @@ function RegisterExtractorPack(id) if (not match) then Log(1, 'Dotnet subcommand detected: %s', arg) end - if arg == 'build' or arg == 'msbuild' or arg == 'publish' or arg == 'pack' or arg == 'test' then + if arg == 'build' or arg == 'msbuild' or arg == 'publish' or arg == 'pack' then match = true break end @@ -48,6 +49,14 @@ function RegisterExtractorPack(id) dotnetRunNeedsSeparator = true dotnetRunInjectionIndex = i + 1 end + if arg == 'test' then + match = true + end + -- for `dotnet test`, we should not append `-p:UseSharedCompilation=false` to the command line + -- if a library or executable is being provided as an argument. + if arg:match('%.exe$') or arg:match('%.dll') then + libOrExe = true + end end -- if we see a separator to `dotnet run`, inject just prior to the existing separator if arg == '--' then @@ -62,7 +71,7 @@ function RegisterExtractorPack(id) dotnetRunInjectionIndex = i end end - if match then + if match and not libOrExe then local injections = { '-p:UseSharedCompilation=false', '-p:EmitCompilerGeneratedFiles=true' } if dotnetRunNeedsSeparator then table.insert(injections, '--') From 5c9b0b9b76552929c283e539553dd75f38a053b8 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 7 Jun 2023 09:49:23 +0200 Subject: [PATCH 3/8] C#: Address review comments. --- .../integration-tests/posix-only/dotnet_test/test.py | 2 +- csharp/tools/tracing-config.lua | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/csharp/ql/integration-tests/posix-only/dotnet_test/test.py b/csharp/ql/integration-tests/posix-only/dotnet_test/test.py index 1012454003b5..f69d01b2188d 100644 --- a/csharp/ql/integration-tests/posix-only/dotnet_test/test.py +++ b/csharp/ql/integration-tests/posix-only/dotnet_test/test.py @@ -7,4 +7,4 @@ # Explicitly build and then run tests. run_codeql_database_create(['dotnet clean', 'rm -rf test-db', 'dotnet build -o myout', 'dotnet test myout/dotnet_test.dll'], test_db="test2-db", lang="csharp") -check_diagnostics(test_db="test2-db") \ No newline at end of file +check_diagnostics(test_db="test2-db") diff --git a/csharp/tools/tracing-config.lua b/csharp/tools/tracing-config.lua index 7fa03af2de13..5ba1a078079f 100644 --- a/csharp/tools/tracing-config.lua +++ b/csharp/tools/tracing-config.lua @@ -21,9 +21,9 @@ function RegisterExtractorPack(id) -- if that's `build`, we append `-p:UseSharedCompilation=false` to the command line, -- otherwise we do nothing. local match = false + local testMatch = false; local dotnetRunNeedsSeparator = false; local dotnetRunInjectionIndex = nil; - local libOrExe = false; local argv = compilerArguments.argv if OperatingSystem == 'windows' then -- let's hope that this split matches the escaping rules `dotnet` applies to command line arguments @@ -35,7 +35,7 @@ function RegisterExtractorPack(id) -- dotnet options start with either - or / (both are legal) local firstCharacter = string.sub(arg, 1, 1) if not (firstCharacter == '-') and not (firstCharacter == '/') then - if (not match) then + if (not match and not testMatch) then Log(1, 'Dotnet subcommand detected: %s', arg) end if arg == 'build' or arg == 'msbuild' or arg == 'publish' or arg == 'pack' then @@ -50,12 +50,12 @@ function RegisterExtractorPack(id) dotnetRunInjectionIndex = i + 1 end if arg == 'test' then - match = true + testMatch = true end -- for `dotnet test`, we should not append `-p:UseSharedCompilation=false` to the command line -- if a library or executable is being provided as an argument. - if arg:match('%.exe$') or arg:match('%.dll') then - libOrExe = true + if testMatch and (arg:match('%.exe$') or arg:match('%.dll')) then + testMatch = false end end -- if we see a separator to `dotnet run`, inject just prior to the existing separator @@ -71,7 +71,7 @@ function RegisterExtractorPack(id) dotnetRunInjectionIndex = i end end - if match and not libOrExe then + if match or testMatch then local injections = { '-p:UseSharedCompilation=false', '-p:EmitCompilerGeneratedFiles=true' } if dotnetRunNeedsSeparator then table.insert(injections, '--') From 3eb3178ba5d045a4b39e65a8f8948d2b10c01e62 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 7 Jun 2023 10:10:51 +0200 Subject: [PATCH 4/8] C#: Add change note. --- csharp/ql/lib/change-notes/2023-06-06-dotnettest.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 csharp/ql/lib/change-notes/2023-06-06-dotnettest.md diff --git a/csharp/ql/lib/change-notes/2023-06-06-dotnettest.md b/csharp/ql/lib/change-notes/2023-06-06-dotnettest.md new file mode 100644 index 000000000000..8e04877da1ee --- /dev/null +++ b/csharp/ql/lib/change-notes/2023-06-06-dotnettest.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* C#: Update the LUA tracer such that `-p:SharedCompilation=false` is not injected when `dotnet test` is applied to a `dll` or `exe` file. \ No newline at end of file From d4d571e43569d281b01bb797f871a952f9cbe984 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 7 Jun 2023 12:44:36 +0200 Subject: [PATCH 5/8] C#: Better change note. Co-authored-by: Michael B. Gale --- csharp/ql/lib/change-notes/2023-06-06-dotnettest.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/csharp/ql/lib/change-notes/2023-06-06-dotnettest.md b/csharp/ql/lib/change-notes/2023-06-06-dotnettest.md index 8e04877da1ee..e7179b93189a 100644 --- a/csharp/ql/lib/change-notes/2023-06-06-dotnettest.md +++ b/csharp/ql/lib/change-notes/2023-06-06-dotnettest.md @@ -1,4 +1,4 @@ --- category: minorAnalysis --- -* C#: Update the LUA tracer such that `-p:SharedCompilation=false` is not injected when `dotnet test` is applied to a `dll` or `exe` file. \ No newline at end of file +* C#: Analysis of the `dotnet test` command supplied with a `dll` or `exe` file as argument no longer fails due to the addition of an erroneous `-p:SharedCompilation=false` argument. \ No newline at end of file From f9c890be3548a0e4eb25b75fa5aedbf2b00a9c54 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 7 Jun 2023 14:49:04 +0200 Subject: [PATCH 6/8] C#: Address review comments. --- csharp/tools/tracing-config.lua | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/csharp/tools/tracing-config.lua b/csharp/tools/tracing-config.lua index 5ba1a078079f..49044226da3b 100644 --- a/csharp/tools/tracing-config.lua +++ b/csharp/tools/tracing-config.lua @@ -21,7 +21,6 @@ function RegisterExtractorPack(id) -- if that's `build`, we append `-p:UseSharedCompilation=false` to the command line, -- otherwise we do nothing. local match = false - local testMatch = false; local dotnetRunNeedsSeparator = false; local dotnetRunInjectionIndex = nil; local argv = compilerArguments.argv @@ -35,7 +34,7 @@ function RegisterExtractorPack(id) -- dotnet options start with either - or / (both are legal) local firstCharacter = string.sub(arg, 1, 1) if not (firstCharacter == '-') and not (firstCharacter == '/') then - if (not match and not testMatch) then + if (not match) then Log(1, 'Dotnet subcommand detected: %s', arg) end if arg == 'build' or arg == 'msbuild' or arg == 'publish' or arg == 'pack' then @@ -50,12 +49,13 @@ function RegisterExtractorPack(id) dotnetRunInjectionIndex = i + 1 end if arg == 'test' then - testMatch = true + match = true end - -- for `dotnet test`, we should not append `-p:UseSharedCompilation=false` to the command line + -- for `dotnet [test|run]`, we should not append `-p:UseSharedCompilation=false` to the command line -- if a library or executable is being provided as an argument. - if testMatch and (arg:match('%.exe$') or arg:match('%.dll')) then - testMatch = false + if arg:match('%.exe$') or arg:match('%.dll') then + match = false + break end end -- if we see a separator to `dotnet run`, inject just prior to the existing separator @@ -71,7 +71,7 @@ function RegisterExtractorPack(id) dotnetRunInjectionIndex = i end end - if match or testMatch then + if match then local injections = { '-p:UseSharedCompilation=false', '-p:EmitCompilerGeneratedFiles=true' } if dotnetRunNeedsSeparator then table.insert(injections, '--') From 65e651506ce0291c9e42dd936c4c3a43e76a74a5 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Thu, 8 Jun 2023 08:51:21 +0200 Subject: [PATCH 7/8] C#: Address review comments. --- csharp/tools/tracing-config.lua | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/csharp/tools/tracing-config.lua b/csharp/tools/tracing-config.lua index 49044226da3b..f04169caff50 100644 --- a/csharp/tools/tracing-config.lua +++ b/csharp/tools/tracing-config.lua @@ -21,6 +21,7 @@ function RegisterExtractorPack(id) -- if that's `build`, we append `-p:UseSharedCompilation=false` to the command line, -- otherwise we do nothing. local match = false + local testMatch = false local dotnetRunNeedsSeparator = false; local dotnetRunInjectionIndex = nil; local argv = compilerArguments.argv @@ -50,10 +51,11 @@ function RegisterExtractorPack(id) end if arg == 'test' then match = true + testMatch = true end - -- for `dotnet [test|run]`, we should not append `-p:UseSharedCompilation=false` to the command line - -- if a library or executable is being provided as an argument. - if arg:match('%.exe$') or arg:match('%.dll') then + -- for `dotnet test`, we should not append `-p:UseSharedCompilation=false` to the command line + -- if an `exe` or `dll` is passed as an argument as the call is forwarded to vstest. + if testMatch and (arg:match('%.exe$') or arg:match('%.dll')) then match = false break end From 2fece9d72148593f39a770c5660ddd1f93bb1c44 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Thu, 8 Jun 2023 09:58:52 +0200 Subject: [PATCH 8/8] C#: Add MSTEST test project and check that the call to vstest doesn't get the UseSharedCompilation=false flag forwarded. --- .../dotnet_test_mstest/UnitTest1.cs | 10 ++++++++++ .../posix-only/dotnet_test_mstest/Usings.cs | 1 + .../dotnet_test_mstest.csproj | 19 +++++++++++++++++++ .../posix-only/dotnet_test_mstest/test.py | 10 ++++++++++ 4 files changed, 40 insertions(+) create mode 100644 csharp/ql/integration-tests/posix-only/dotnet_test_mstest/UnitTest1.cs create mode 100644 csharp/ql/integration-tests/posix-only/dotnet_test_mstest/Usings.cs create mode 100644 csharp/ql/integration-tests/posix-only/dotnet_test_mstest/dotnet_test_mstest.csproj create mode 100644 csharp/ql/integration-tests/posix-only/dotnet_test_mstest/test.py diff --git a/csharp/ql/integration-tests/posix-only/dotnet_test_mstest/UnitTest1.cs b/csharp/ql/integration-tests/posix-only/dotnet_test_mstest/UnitTest1.cs new file mode 100644 index 000000000000..7e3b2ce1d1c3 --- /dev/null +++ b/csharp/ql/integration-tests/posix-only/dotnet_test_mstest/UnitTest1.cs @@ -0,0 +1,10 @@ +namespace dotnet_test_mstest; + +[TestClass] +public class UnitTest1 +{ + [TestMethod] + public void TestMethod1() + { + } +} diff --git a/csharp/ql/integration-tests/posix-only/dotnet_test_mstest/Usings.cs b/csharp/ql/integration-tests/posix-only/dotnet_test_mstest/Usings.cs new file mode 100644 index 000000000000..540383dcf43c --- /dev/null +++ b/csharp/ql/integration-tests/posix-only/dotnet_test_mstest/Usings.cs @@ -0,0 +1 @@ +global using Microsoft.VisualStudio.TestTools.UnitTesting; diff --git a/csharp/ql/integration-tests/posix-only/dotnet_test_mstest/dotnet_test_mstest.csproj b/csharp/ql/integration-tests/posix-only/dotnet_test_mstest/dotnet_test_mstest.csproj new file mode 100644 index 000000000000..95c7586e04eb --- /dev/null +++ b/csharp/ql/integration-tests/posix-only/dotnet_test_mstest/dotnet_test_mstest.csproj @@ -0,0 +1,19 @@ + + + + net7.0 + enable + enable + + false + Exe + + + + + + + + + + diff --git a/csharp/ql/integration-tests/posix-only/dotnet_test_mstest/test.py b/csharp/ql/integration-tests/posix-only/dotnet_test_mstest/test.py new file mode 100644 index 000000000000..345d26a15169 --- /dev/null +++ b/csharp/ql/integration-tests/posix-only/dotnet_test_mstest/test.py @@ -0,0 +1,10 @@ +from create_database_utils import * +from diagnostics_test_utils import * + +# Implicitly build and then run tests. +run_codeql_database_create(['dotnet test'], test_db="test-db", lang="csharp") +check_diagnostics() + +# Explicitly build and then run tests. +run_codeql_database_create(['dotnet clean', 'rm -rf test-db', 'dotnet build -o myout --os win', 'dotnet test myout/dotnet_test_mstest.exe'], test_db="test2-db", lang="csharp") +check_diagnostics(test_db="test2-db")