Fixed self-ref type detection. (#6455) (#6456)

crates/cairo-lang-language-server/src/lib.rs

@@ -359,9 +359,6 @@ impl Backend {
         self.state_mutex.lock().await
     }
 
-    // TODO(spapini): Consider managing vfs in a different way, using the
-    // client.send_notification::<UpdateVirtualFile> call.
-
     /// Refresh diagnostics and send diffs to client.
     #[tracing::instrument(level = "debug", skip_all)]
     async fn refresh_diagnostics(&self) -> LSPResult<()> {

crates/cairo-lang-language-server/src/project/scarb.rs

@@ -5,8 +5,7 @@ use std::path::Path;
 use anyhow::{bail, ensure, Context, Result};
 use cairo_lang_filesystem::db::{CrateSettings, Edition, ExperimentalFeaturesConfig};
 use itertools::Itertools;
-use scarb_metadata::{CompilationUnitMetadata, Metadata, PackageMetadata};
-use serde_json::Value;
+use scarb_metadata::{Metadata, PackageMetadata};
 use tracing::{debug, error, warn};
 
 use crate::lang::db::AnalysisDatabase;
@@ -41,26 +40,16 @@ pub fn update_crate_roots(metadata: &Metadata, db: &mut AnalysisDatabase) {
             let mut package =
                 metadata.packages.iter().find(|package| package.id == component.package);
 
-            // TODO(pmagiera): this is a hack, remove this when `scarb metadata` is fixed.
-            if package.is_none()
-                && is_integration_test_cu(compilation_unit)
-                && compilation_unit.package == component.package
-            {
-                let human_readable_member_name = component
-                    .package
-                    .repr
-                    .split("_integrationtest")
-                    .collect::<Vec<_>>()
-                    .first()
-                    // NOTE: this `unwrap` cannot fail since `split` always returns a non-empty
-                    // vector.
-                    .unwrap()
-                    .to_string();
-
+            // For some compilation units corresponding to integration tests,
+            // a main package of the compilation unit may not be found in a list of packages
+            // (metadata hack, intended by scarb).
+            // We instead find a package that specifies the target of the compilation unit
+            // and later use it to extract edition and experimental features.
+            if package.is_none() && compilation_unit.package == component.package {
                 package = metadata
                     .packages
                     .iter()
-                    .find(|package| package.id.repr.starts_with(&human_readable_member_name));
+                    .find(|p| p.targets.iter().any(|t| t.name == compilation_unit.target.name));
             }
 
             if package.is_none() {
@@ -268,9 +257,3 @@ fn scarb_package_experimental_features(
         coupons: contains("coupons"),
     }
 }
-
-fn is_integration_test_cu(compilation_unit: &CompilationUnitMetadata) -> bool {
-    compilation_unit.target.kind == "test"
-        && compilation_unit.target.params.get("test-type").and_then(Value::as_str)
-            == Some("integration")
-}

crates/cairo-lang-lowering/src/lower/lower_match.rs

@@ -744,7 +744,7 @@ pub(crate) fn lower_concrete_enum_match(
     )?;
     let mut arm_var_ids = vec![];
     let mut block_ids = vec![];
-    let varinats_block_builders = concrete_variants
+    let variants_block_builders = concrete_variants
         .iter()
         .map(|concrete_variant| {
             let PatternPath { arm_index, pattern_index } = variant_map
@@ -836,7 +836,7 @@ pub(crate) fn lower_concrete_enum_match(
         empty_match_info,
         location,
         arms,
-        varinats_block_builders,
+        variants_block_builders,
         match_type,
     )?;
 
@@ -887,7 +887,7 @@ pub(crate) fn lower_optimized_extern_match(
     let mut arm_var_ids = vec![];
     let mut block_ids = vec![];
 
-    let varinats_block_builders = concrete_variants
+    let variants_block_builders = concrete_variants
         .iter()
         .map(|concrete_variant| {
             let mut subscope = create_subscope(ctx, builder);
@@ -969,7 +969,7 @@ pub(crate) fn lower_optimized_extern_match(
         empty_match_info,
         location,
         match_arms,
-        varinats_block_builders,
+        variants_block_builders,
         match_type,
     )?;
     let match_info = MatchInfo::Extern(MatchExternInfo {
@@ -1000,10 +1000,10 @@ fn group_match_arms(
     empty_match_info: MatchInfo,
     location: LocationId,
     arms: &[MatchArmWrapper],
-    varinats_block_builders: Vec<MatchLeafBuilder>,
+    variants_block_builders: Vec<MatchLeafBuilder>,
     kind: MatchKind,
 ) -> LoweringResult<Vec<SealedBlockBuilder>> {
-    varinats_block_builders
+    variants_block_builders
         .into_iter()
         .sorted_by_key(|MatchLeafBuilder { arm_index, .. }| *arm_index)
         .group_by(|MatchLeafBuilder { arm_index, .. }| *arm_index)

crates/cairo-lang-plugins/src/test_data/derive

@@ -412,7 +412,7 @@ struct UnsupportedTrait {}
 struct NonSimplePath {}
 
 #[derive(Clone)]
-extern type NotClonable;
+extern type NotCloneable;
 
 #[derive(Default)]
 enum NoDefaultValue {
@@ -442,7 +442,7 @@ struct UnsupportedTrait {}
 struct NonSimplePath {}
 
 #[derive(Clone)]
-extern type NotClonable;
+extern type NotCloneable;
 
 #[derive(Default)]
 enum NoDefaultValue {

crates/cairo-lang-sierra-generator/src/db.rs

@@ -79,7 +79,6 @@ pub trait SierraGenGroup: LoweringGroup + Upcast<dyn LoweringGroup> {
 
     /// Returns if the semantic id has a circular definition.
     #[salsa::invoke(crate::types::is_self_referential)]
-    #[salsa::cycle(crate::types::is_self_referential_cycle)]
     fn is_self_referential(&self, type_id: semantic::TypeId) -> Maybe<bool>;
 
     /// Returns the semantic type ids the type is directly dependent on.
@@ -89,6 +88,10 @@ pub trait SierraGenGroup: LoweringGroup + Upcast<dyn LoweringGroup> {
     #[salsa::invoke(crate::types::type_dependencies)]
     fn type_dependencies(&self, type_id: semantic::TypeId) -> Maybe<Arc<[semantic::TypeId]>>;
 
+    #[salsa::invoke(crate::types::has_in_deps)]
+    #[salsa::cycle(crate::types::has_in_deps_cycle)]
+    fn has_in_deps(&self, type_id: semantic::TypeId, needle: semantic::TypeId) -> Maybe<bool>;
+
     /// Returns the [cairo_lang_sierra::program::FunctionSignature] object for the given function
     /// id.
     fn get_function_signature(

crates/cairo-lang-sierra-generator/src/types.rs

@@ -161,21 +161,9 @@ pub fn get_concrete_long_type_id(
     })
 }
 
-/// See [SierraGenGroup::is_self_referential] for documentation.
-pub fn is_self_referential_cycle(
-    _db: &dyn SierraGenGroup,
-    _cycle: &salsa::Cycle,
-    _type_id: &semantic::TypeId,
-) -> Maybe<bool> {
-    Ok(true)
-}
-
 /// See [SierraGenGroup::is_self_referential] for documentation.
 pub fn is_self_referential(db: &dyn SierraGenGroup, type_id: semantic::TypeId) -> Maybe<bool> {
-    for ty in db.type_dependencies(type_id)?.iter() {
-        db.is_self_referential(*ty)?;
-    }
-    Ok(false)
+    db.has_in_deps(type_id, type_id)
 }
 
 /// See [SierraGenGroup::type_dependencies] for documentation.
@@ -225,3 +213,31 @@ pub fn type_dependencies(
     }
     .into())
 }
+
+/// See [SierraGenGroup::has_in_deps] for documentation.
+pub fn has_in_deps(
+    db: &dyn SierraGenGroup,
+    type_id: semantic::TypeId,
+    needle: semantic::TypeId,
+) -> Maybe<bool> {
+    let deps = type_dependencies(db, type_id)?;
+    if deps.contains(&needle) {
+        return Ok(true);
+    }
+    for dep in deps.iter() {
+        if db.has_in_deps(*dep, needle)? {
+            return Ok(true);
+        }
+    }
+    Ok(false)
+}
+
+/// See [SierraGenGroup::has_in_deps] for documentation.
+pub fn has_in_deps_cycle(
+    _db: &dyn SierraGenGroup,
+    _cycle: &salsa::Cycle,
+    _type_id: &semantic::TypeId,
+    _needle: &semantic::TypeId,
+) -> Maybe<bool> {
+    Ok(false)
+}

tests/bug_samples/issue2249.cairo

@@ -11,3 +11,18 @@ fn simple_test() {
     assert_eq!(bst.value, 12);
     assert!(bst.left.is_none(), "left should be none");
 }
+
+#[derive(Copy, Drop)]
+pub enum RecursiveType {
+    Simple: felt252,
+    Direct: Span<RecursiveType>,
+    ExtraHop: Span<(felt252, RecursiveType)>,
+}
+
+#[inline(never)]
+fn use_value(_value: RecursiveType) {}
+
+#[test]
+fn other_test() {
+    use_value(RecursiveType::Simple(12));
+}

vscode-cairo/src/cairols.ts

@@ -95,22 +95,29 @@ export async function setupLanguageServer(ctx: Context): Promise<lc.LanguageClie
   client.onNotification(
     new lc.NotificationType<string>("cairo/corelib-version-mismatch"),
     async (errorMessage) => {
-      const reloadWindow = "Reload window";
+      const restart = "Restart CairoLS";
       const cleanScarbCache = "Clean Scarb cache and reload";
 
       const selectedValue = await vscode.window.showErrorMessage(
         errorMessage,
-        reloadWindow,
+        restart,
         cleanScarbCache,
       );
 
+      const restartLS = async () => {
+        const client = weakClient.deref();
+        if (client) {
+          await client.restart();
+        }
+      };
+
       switch (selectedValue) {
-        case reloadWindow:
-          await vscode.commands.executeCommand("workbench.action.reloadWindow");
+        case restart:
+          await restartLS();
           break;
         case cleanScarbCache:
           await scarb?.cacheClean(ctx);
-          await vscode.commands.executeCommand("workbench.action.reloadWindow");
+          await restartLS();
           break;
       }
     },

vscode-cairo/src/textDocumentProviders.ts

@@ -4,8 +4,6 @@ import { Context } from "./context";
 import { expandMacro, vfsProvide, viewAnalyzedCrates } from "./lspRequests";
 
 export const registerVfsProvider = (client: lc.LanguageClient, ctx: Context) => {
-  const eventEmitter = new vscode.EventEmitter<vscode.Uri>();
-
   const vfsProvider: vscode.TextDocumentContentProvider = {
     async provideTextDocumentContent(uri: vscode.Uri): Promise<string> {
       const res = await client.sendRequest(vfsProvide, {
@@ -14,13 +12,8 @@ export const registerVfsProvider = (client: lc.LanguageClient, ctx: Context) =>
 
       return res.content ?? "";
     },
-    onDidChange: eventEmitter.event,
   };
 
-  client.onNotification("vfs/update", (param) => {
-    eventEmitter.fire(param.uri);
-  });
-
   ctx.extension.subscriptions.push(
     vscode.workspace.registerTextDocumentContentProvider("vfs", vfsProvider),
   );