Merge branch 'main' into oscarsj/test-cpp-internal-checks

Rust: pick correct edition for the files

cpp/ql/src/Critical/DoubleFreeBad.cpp

@@ -1,10 +1,13 @@
+
 int* f() {
 	int *buff = malloc(SIZE*sizeof(int));
 	do_stuff(buff);
 	free(buff);
 	int *new_buffer = malloc(SIZE*sizeof(int));
-	free(buff); // BAD: If new_buffer is assigned the same address as buff,
+	free(buff);
-              // the memory allocator will free the new buffer memory region,
+	// BAD: If new_buffer is assigned the same address as buff,
-              // leading to use-after-free problems and memory corruption.
+    // the memory allocator will free the new buffer memory region,
+    // leading to use-after-free problems and memory corruption.
+	// abc
 	return new_buffer;
 }

csharp/autobuilder/Semmle.Autobuild.CSharp.Tests/BuildScripts.cs

@@ -424,8 +424,7 @@ namespace Semmle.Autobuild.CSharp.Tests
             return new CSharpAutobuilder(actions, options);
         }
 
-        [Fact]
+        private void SetupActionForDotnet()
-        public void TestDefaultCSharpAutoBuilder()
         {
             actions.RunProcess["cmd.exe /C dotnet --info"] = 0;
             actions.RunProcess[@"cmd.exe /C dotnet clean C:\Project\test.csproj"] = 0;
@@ -438,20 +437,80 @@ namespace Semmle.Autobuild.CSharp.Tests
             actions.GetEnvironmentVariable["CODEQL_EXTRACTOR_CSHARP_SCRATCH_DIR"] = "scratch";
             actions.EnumerateFiles[@"C:\Project"] = "foo.cs\nbar.cs\ntest.csproj";
             actions.EnumerateDirectories[@"C:\Project"] = "";
-            var xml = new XmlDocument();
+        }
-            xml.LoadXml(@"<Project Sdk=""Microsoft.NET.Sdk"">
-  <PropertyGroup>
-    <OutputType>Exe</OutputType>
-    <TargetFramework>netcoreapp2.1</TargetFramework>
-  </PropertyGroup>
 
-</Project>");
+        private void CreateAndVerifyDotnetScript(XmlDocument xml)
+        {
             actions.LoadXml[@"C:\Project\test.csproj"] = xml;
 
             var autobuilder = CreateAutoBuilder(true);
             TestAutobuilderScript(autobuilder, 0, 4);
         }
 
+        [Fact]
+        public void TestDefaultCSharpAutoBuilder1()
+        {
+            SetupActionForDotnet();
+            var xml = new XmlDocument();
+            xml.LoadXml(
+            """
+            <Project Sdk="Microsoft.NET.Sdk">
+              <PropertyGroup>
+                <OutputType>Exe</OutputType>
+                <TargetFramework>netcoreapp2.1</TargetFramework>
+              </PropertyGroup>
+            </Project>
+            """);
+            CreateAndVerifyDotnetScript(xml);
+        }
+
+        [Fact]
+        public void TestDefaultCSharpAutoBuilder2()
+        {
+            SetupActionForDotnet();
+            var xml = new XmlDocument();
+
+            xml.LoadXml(
+            """
+            <Project>
+              <Sdk Name="Microsoft.NET.Sdk" />
+
+              <PropertyGroup>
+                <OutputType>Exe</OutputType>
+                <TargetFramework>net9.0</TargetFramework>
+                <ImplicitUsings>enable</ImplicitUsings>
+                <Nullable>enable</Nullable>
+              </PropertyGroup>
+            </Project>
+            """
+            );
+            CreateAndVerifyDotnetScript(xml);
+        }
+
+        [Fact]
+        public void TestDefaultCSharpAutoBuilder3()
+        {
+            SetupActionForDotnet();
+            var xml = new XmlDocument();
+
+            xml.LoadXml(
+            """
+            <Project>
+              <Import Project="Sdk.props" Sdk="Microsoft.NET.Sdk" />
+
+              <PropertyGroup>
+                <OutputType>Exe</OutputType>
+                <TargetFramework>net9.0</TargetFramework>
+                <ImplicitUsings>enable</ImplicitUsings>
+                <Nullable>enable</Nullable>
+              </PropertyGroup>
+              <Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" />
+            </Project>
+            """
+            );
+            CreateAndVerifyDotnetScript(xml);
+        }
+
         [Fact]
         public void TestLinuxCSharpAutoBuilder()
         {

csharp/autobuilder/Semmle.Autobuild.Shared/Project.cs

@@ -3,7 +3,6 @@ using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using System.Xml;
-using Semmle.Util.Logging;
 
 namespace Semmle.Autobuild.Shared
 {
@@ -26,6 +25,26 @@ namespace Semmle.Autobuild.Shared
         private readonly Lazy<List<Project<TAutobuildOptions>>> includedProjectsLazy;
         public override IEnumerable<IProjectOrSolution> IncludedProjects => includedProjectsLazy.Value;
 
+        private static bool HasSdkAttribute(XmlElement xml) =>
+            xml.HasAttribute("Sdk");
+
+        private static bool AnyElement(XmlNodeList l, Func<XmlElement, bool> f) =>
+            l.OfType<XmlElement>().Any(f);
+
+        /// <summary>
+        /// According to https://learn.microsoft.com/en-us/visualstudio/msbuild/how-to-use-project-sdk?view=vs-2022#reference-a-project-sdk
+        /// there are three ways to reference a project SDK:
+        ///  1. As an attribute on the <Project/>.
+        ///  2. As a top level element of <Project>.
+        ///  3. As an attribute on an <Import> element.
+        ///
+        /// Returns true, if the Sdk attribute is used, otherwise false.
+        /// </summary>
+        private static bool ReferencesSdk(XmlElement xml) =>
+            HasSdkAttribute(xml) || // Case 1
+            AnyElement(xml.ChildNodes, e => e.Name == "Sdk") || // Case 2
+            AnyElement(xml.GetElementsByTagName("Import"), HasSdkAttribute); // Case 3
+
         public Project(Autobuilder<TAutobuildOptions> builder, string path) : base(builder, path)
         {
             ToolsVersion = new Version();
@@ -49,7 +68,7 @@ namespace Semmle.Autobuild.Shared
 
             if (root?.Name == "Project")
             {
-                if (root.HasAttribute("Sdk"))
+                if (ReferencesSdk(root))
                 {
                     DotNetProject = true;
                     return;

csharp/ql/lib/change-notes/2025-04-11-auto-builder-sdk.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Improved autobuilder logic for detecting whether a project references a SDK (and should be built using `dotnet`).

rust/extractor/src/rust_analyzer.rs

@@ -1,5 +1,5 @@
 use itertools::Itertools;
-use ra_ap_base_db::{EditionedFileId, RootQueryDb, SourceDatabase};
+use ra_ap_base_db::{EditionedFileId, FileText, RootQueryDb, SourceDatabase};
 use ra_ap_hir::Semantics;
 use ra_ap_ide_db::RootDatabase;
 use ra_ap_load_cargo::{LoadCargoConfig, load_workspace_at};
@@ -7,7 +7,6 @@ use ra_ap_paths::{AbsPath, Utf8PathBuf};
 use ra_ap_project_model::ProjectManifest;
 use ra_ap_project_model::{CargoConfig, ManifestPath};
 use ra_ap_span::Edition;
-use ra_ap_span::EditionedFileId as SpanEditionedFileId;
 use ra_ap_span::TextRange;
 use ra_ap_span::TextSize;
 use ra_ap_syntax::SourceFile;
@@ -54,7 +53,6 @@ impl<'a> RustAnalyzer<'a> {
     ) -> Option<(RootDatabase, Vfs)> {
         let progress = |t| (trace!("progress: {}", t));
         let manifest = project.manifest_path();
-
         match load_workspace_at(manifest.as_ref(), config, load_config, &progress) {
             Ok((db, vfs, _macro_server)) => Some((db, vfs)),
             Err(err) => {
@@ -66,67 +64,70 @@ impl<'a> RustAnalyzer<'a> {
     pub fn new(vfs: &'a Vfs, semantics: &'a Semantics<'a, RootDatabase>) -> Self {
         RustAnalyzer::WithSemantics { vfs, semantics }
     }
-    pub fn parse(&self, path: &Path) -> ParseResult {
+    fn get_file_data(
-        let no_semantics_reason;
+        &self,
+        path: &Path,
+    ) -> Result<(&Semantics<RootDatabase>, EditionedFileId, FileText), &str> {
         match self {
+            RustAnalyzer::WithoutSemantics { reason } => Err(reason),
             RustAnalyzer::WithSemantics { vfs, semantics } => {
-                if let Some(file_id) = path_to_file_id(path, vfs) {
+                let file_id = path_to_file_id(path, vfs).ok_or("file not found in project")?;
-                    if let Ok(input) = std::panic::catch_unwind(|| semantics.db.file_text(file_id))
+                let input = std::panic::catch_unwind(|| semantics.db.file_text(file_id))
-                    {
+                    .or(Err("no text available for the file in the project"))?;
-                        let file_id = EditionedFileId::new(
+                let editioned_file_id = semantics
-                            semantics.db,
+                    .attach_first_edition(file_id)
-                            SpanEditionedFileId::current_edition(file_id),
+                    .ok_or("failed to determine rust edition")?;
-                        );
+                Ok((
-                        let source_file = semantics.parse(file_id);
+                    semantics,
-                        let errors = semantics
+                    EditionedFileId::new(semantics.db, editioned_file_id),
-                            .db
+                    input,
-                            .parse_errors(file_id)
+                ))
-                            .into_iter()
-                            .flat_map(|x| x.to_vec())
-                            .collect();
-
-                        return ParseResult {
-                            ast: source_file,
-                            text: input.text(semantics.db),
-                            errors,
-                            semantics_info: Ok(FileSemanticInformation { file_id, semantics }),
-                        };
-                    }
-                    debug!(
-                        "No text available for file_id '{:?}', falling back to loading file '{}' from disk.",
-                        file_id,
-                        path.to_string_lossy()
-                    );
-                    no_semantics_reason = "no text available for the file in the project";
-                } else {
-                    no_semantics_reason = "file not found in project";
-                }
-            }
-            RustAnalyzer::WithoutSemantics { reason } => {
-                no_semantics_reason = reason;
             }
         }
-        let mut errors = Vec::new();
+    }
-        let input = match std::fs::read(path) {
-            Ok(data) => data,
-            Err(e) => {
-                errors.push(SyntaxError::new(
-                    format!("Could not read {}: {}", path.to_string_lossy(), e),
-                    TextRange::empty(TextSize::default()),
-                ));
-                vec![]
-            }
-        };
-        let (input, err) = from_utf8_lossy(&input);
 
-        let parse = ra_ap_syntax::ast::SourceFile::parse(&input, Edition::CURRENT);
+    pub fn parse(&self, path: &Path) -> ParseResult {
-        errors.extend(parse.errors());
+        match self.get_file_data(path) {
-        errors.extend(err);
+            Ok((semantics, file_id, input)) => {
-        ParseResult {
+                let source_file = semantics.parse(file_id);
-            ast: parse.tree(),
+                let errors = semantics
-            text: input.as_ref().into(),
+                    .db
-            errors,
+                    .parse_errors(file_id)
-            semantics_info: Err(no_semantics_reason),
+                    .into_iter()
+                    .flat_map(|x| x.to_vec())
+                    .collect();
+
+                ParseResult {
+                    ast: source_file,
+                    text: input.text(semantics.db),
+                    errors,
+                    semantics_info: Ok(FileSemanticInformation { file_id, semantics }),
+                }
+            }
+            Err(reason) => {
+                let mut errors = Vec::new();
+                let input = match std::fs::read(path) {
+                    Ok(data) => data,
+                    Err(e) => {
+                        errors.push(SyntaxError::new(
+                            format!("Could not read {}: {}", path.to_string_lossy(), e),
+                            TextRange::empty(TextSize::default()),
+                        ));
+                        vec![]
+                    }
+                };
+                let (input, err) = from_utf8_lossy(&input);
+
+                let parse = ra_ap_syntax::ast::SourceFile::parse(&input, Edition::CURRENT);
+                errors.extend(parse.errors());
+                errors.extend(err);
+                ParseResult {
+                    ast: parse.tree(),
+                    text: input.as_ref().into(),
+                    errors,
+                    semantics_info: Err(reason),
+                }
+            }
         }
     }
 }
@@ -173,8 +174,10 @@ impl TomlReader {
 }
 
 fn workspace_members_match(workspace_dir: &AbsPath, members: &[String], target: &AbsPath) -> bool {
-    members.iter().any(|p| {
+    target.strip_prefix(workspace_dir).is_some_and(|rel_path| {
-        glob::Pattern::new(workspace_dir.join(p).as_str()).is_ok_and(|p| p.matches(target.as_str()))
+        members
+            .iter()
+            .any(|p| glob::Pattern::new(p).is_ok_and(|p| p.matches(rel_path.as_str())))
     })
 }
 

rust/ql/integration-tests/conftest.py

@@ -2,13 +2,35 @@ import pytest
 import json
 import commands
 import pathlib
+import tomllib
+
+
+@pytest.fixture(params=[2018, 2021, 2024])
+def rust_edition(request):
+    return request.param
 
 
 @pytest.fixture
-def cargo(cwd):
+def cargo(cwd, rust_edition):
-    assert (cwd / "Cargo.toml").exists()
+    manifest_file = cwd / "Cargo.toml"
+    assert manifest_file.exists()
     (cwd / "rust-project.json").unlink(missing_ok=True)
 
+    def update(file):
+        contents = file.read_text()
+        m = tomllib.loads(contents)
+        if 'package' in m:
+            # tomllib does not support writing, and we don't want to use further dependencies
+            # so we just do a dumb search and replace
+            contents = contents.replace(f'edition = "{m["package"]["edition"]}"', f'edition = "{rust_edition}"')
+            file.write_text(contents)
+        if 'members' in m.get('workspace', ()):
+            for member in m['workspace']['members']:
+                update(file.parent / member / "Cargo.toml")
+
+    update(manifest_file)
+
+
 @pytest.fixture(scope="session")
 def rust_sysroot_src() -> str:
     rust_sysroot = pathlib.Path(commands.run("rustc --print sysroot", _capture=True))
@@ -16,15 +38,19 @@ def rust_sysroot_src() -> str:
     assert ret.exists()
     return str(ret)
 
+
 @pytest.fixture
-def rust_project(cwd, rust_sysroot_src):
+def rust_project(cwd, rust_sysroot_src, rust_edition):
     project_file = cwd / "rust-project.json"
     assert project_file.exists()
     project = json.loads(project_file.read_text())
     project["sysroot_src"] = rust_sysroot_src
+    for c in project["crates"]:
+        c["edition"] = str(rust_edition)
     project_file.write_text(json.dumps(project, indent=4))
     (cwd / "Cargo.toml").unlink(missing_ok=True)
 
+
 @pytest.fixture
 def rust_check_diagnostics(check_diagnostics):
     check_diagnostics.redact += [

rust/ql/integration-tests/hello-project/Cargo.toml

@@ -2,6 +2,6 @@
 [package]
 name = "hello-cargo"
 version = "0.1.0"
-edition = "2021"
+edition = "2021"  # replaced in test
 
 [dependencies]

rust/ql/integration-tests/hello-workspace/exe/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "exe"
 version = "0.1.0"
-edition = "2021"
+edition = "2021"  # replaced in test
 
 [dependencies]
 lib = { path = "../lib" }

rust/ql/integration-tests/hello-workspace/lib/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lib"
 version = "0.1.0"
-edition = "2021"
+edition = "2021"  # replaced in test
 
 [dependencies]

rust/ql/integration-tests/hello-workspace/rust-project.json

@@ -21,4 +21,4 @@
       "deps": []
     }
   ]
-}
+}

rust/ql/integration-tests/hello-workspace/test_workspace.py

@@ -1,6 +1,5 @@
 import pytest
 
-
 @pytest.mark.ql_test("steps.ql", expected=".cargo.expected")
 @pytest.mark.ql_test("summary.qlref", expected=".cargo.expected")
 def test_cargo(codeql, rust, cargo, check_source_archive, rust_check_diagnostics):