Skip to content

Commit 7182870

Browse files
committed
Revert "Represent a package in Proc Macro Server more reliably"
This reverts commit 00c0f57.
1 parent 9c1873c commit 7182870

File tree

11 files changed

+102
-130
lines changed

11 files changed

+102
-130
lines changed

scarb/src/compiler/compilation_unit.rs

-14
Original file line numberDiff line numberDiff line change
@@ -106,20 +106,6 @@ impl CompilationUnitComponent {
106106
}
107107
}
108108

109-
impl From<&CompilationUnitComponent>
110-
for scarb_proc_macro_server_types::scope::CompilationUnitComponent
111-
{
112-
fn from(value: &CompilationUnitComponent) -> Self {
113-
// `name` and `discriminator` used here must be identital to the scarb-metadata.
114-
// This implementation detail is crucial for communication between PMS and LS.
115-
// Always remember to verify this invariant when changing the internals.
116-
Self {
117-
name: value.cairo_package_name().to_string(),
118-
discriminator: value.id.to_discriminator().map(Into::into),
119-
}
120-
}
121-
}
122-
123109
/// The kind of the compilation unit dependency.
124110
#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)]
125111
pub enum CompilationUnitDependency {

scarb/src/compiler/plugin/collection.rs

+19-20
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ use std::{collections::HashMap, sync::Arc};
33
use anyhow::Result;
44
use cairo_lang_semantic::{inline_macros::get_default_plugin_suite, plugin::PluginSuite};
55
use itertools::Itertools;
6-
use scarb_proc_macro_server_types::scope::CompilationUnitComponent;
76

87
use crate::{
98
compiler::{CairoCompilationUnit, CompilationUnitComponentId, CompilationUnitDependency},
@@ -49,12 +48,17 @@ impl PluginsForComponents {
4948
// NOTE: Since this structure is used to handle JsonRPC requests, its keys have to be serialized to strings.
5049
//
5150
/// A container for Proc Macro Server to manage macros present in the analyzed workspace.
51+
///
52+
/// # Invariant
53+
/// Correct usage of this struct during proc macro server <-> LS communication
54+
/// relies on the implicit contract that keys of `macros_for_packages` are of form
55+
/// `PackageId.to_serialized_string()` which is always equal to
56+
/// `scarb_metadata::CompilationUnitComponentId.repr`.
5257
pub struct WorkspaceProcMacros {
53-
/// A mapping of the form: `cu_component (as a [`CompilationUnitComponent`]) -> plugin`.
54-
/// Contains IDs of all components of all compilation units from the workspace,
55-
/// each mapped to a [`ProcMacroHostPlugin`] which contains
58+
/// A mapping of the form: `package (as a serialized [`PackageId`]) -> plugin`.
59+
/// Contains IDs of all packages from the workspace, each mapped to a [`ProcMacroHostPlugin`] which contains
5660
/// **all proc macro dependencies of the package** collected from **all compilation units it appears in**.
57-
pub macros_for_components: HashMap<CompilationUnitComponent, Arc<ProcMacroHostPlugin>>,
61+
pub macros_for_packages: HashMap<String, Arc<ProcMacroHostPlugin>>,
5862
}
5963

6064
impl WorkspaceProcMacros {
@@ -67,42 +71,37 @@ impl WorkspaceProcMacros {
6771

6872
for &unit in compilation_units {
6973
for (component_id, mut macro_instances) in collect_proc_macros(workspace, unit)? {
70-
let component: CompilationUnitComponent = unit
71-
.components
72-
.iter()
73-
.find(|component| component.id == component_id)
74-
.expect("component should always exist")
75-
.into();
76-
74+
// Here we assume that CompilationUnitComponentId.repr == PackageId.to_serialized_string().
75+
let key = component_id.package_id.to_serialized_string();
7776
macros_for_components
78-
.entry(component)
77+
.entry(key)
7978
.or_default()
8079
.append(&mut macro_instances);
8180
}
8281
}
8382

84-
let macros_for_components = macros_for_components
83+
let macros_for_packages = macros_for_components
8584
.into_iter()
86-
.map(|(component, macro_instances)| {
85+
.map(|(package_id, macro_instances)| {
8786
let deduplicated_instances = macro_instances
8887
.into_iter()
8988
.unique_by(|instance| instance.package_id())
9089
.collect();
9190

9291
let plugin = Arc::new(ProcMacroHostPlugin::try_new(deduplicated_instances)?);
9392

94-
Ok((component, plugin))
93+
Ok((package_id, plugin))
9594
})
9695
.collect::<Result<HashMap<_, _>>>()?;
9796

9897
Ok(Self {
99-
macros_for_components,
98+
macros_for_packages,
10099
})
101100
}
102101

103-
/// Returns a [`ProcMacroHostPlugin`] assigned to the [`CompilationUnitComponent`].
104-
pub fn get(&self, component: &CompilationUnitComponent) -> Option<Arc<ProcMacroHostPlugin>> {
105-
self.macros_for_components.get(component).cloned()
102+
/// Returns a [`ProcMacroHostPlugin`] assigned to the package with `package_id`.
103+
pub fn get(&self, package_id: &str) -> Option<Arc<ProcMacroHostPlugin>> {
104+
self.macros_for_packages.get(package_id).cloned()
106105
}
107106
}
108107

Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
use std::sync::Arc;
1+
use std::{collections::HashMap, sync::Arc};
22

33
use anyhow::Result;
44
use cairo_lang_defs::plugin::MacroPlugin;
55
use convert_case::{Case, Casing};
66
use scarb_proc_macro_server_types::methods::defined_macros::{
7-
CompilationUnitComponentMacros, DefinedMacros, DefinedMacrosResponse,
7+
DefinedMacros, DefinedMacrosResponse, PackageDefinedMacrosInfo,
88
};
99

1010
use super::Handler;
@@ -15,10 +15,10 @@ impl Handler for DefinedMacros {
1515
workspace_macros: Arc<WorkspaceProcMacros>,
1616
_params: Self::Params,
1717
) -> Result<Self::Response> {
18-
let macros_for_cu_components = workspace_macros
19-
.macros_for_components
18+
let macros_by_package_id = workspace_macros
19+
.macros_for_packages
2020
.iter()
21-
.map(|(component, plugin)| {
21+
.map(|(package_id, plugin)| {
2222
let attributes = plugin.declared_attributes_without_executables();
2323
let inline_macros = plugin.declared_inline_macros();
2424
let derives = plugin
@@ -28,18 +28,20 @@ impl Handler for DefinedMacros {
2828
.collect();
2929
let executables = plugin.executable_attributes();
3030

31-
CompilationUnitComponentMacros {
32-
component: component.to_owned(),
33-
attributes,
34-
inline_macros,
35-
derives,
36-
executables,
37-
}
31+
(
32+
package_id.to_owned(),
33+
PackageDefinedMacrosInfo {
34+
attributes,
35+
inline_macros,
36+
derives,
37+
executables,
38+
},
39+
)
3840
})
39-
.collect();
41+
.collect::<HashMap<_, _>>();
4042

4143
Ok(DefinedMacrosResponse {
42-
macros_for_cu_components,
44+
macros_by_package_id,
4345
})
4446
}
4547
}

scarb/src/ops/proc_macro_server/methods/expand_attribute.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ impl Handler for ExpandAttribute {
1919
} = params;
2020

2121
let plugin = workspace_macros
22-
.get(&context.component)
22+
.get(&context.package_id)
2323
.with_context(|| format!("No macros found in scope: {context:?}"))?;
2424

2525
let instance = plugin

scarb/src/ops/proc_macro_server/methods/expand_derive.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ impl Handler for ExpandDerive {
2929
let expansion = Expansion::new(derive.to_case(Case::Snake), ExpansionKind::Derive);
3030

3131
let plugin = workspace_macros
32-
.get(&context.component)
32+
.get(&context.package_id)
3333
.with_context(|| format!("No macros found in scope {context:?}"))?;
3434

3535
let instance = plugin

scarb/src/ops/proc_macro_server/methods/expand_inline.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ impl Handler for ExpandInline {
1919
} = params;
2020

2121
let plugin = workspace_macros
22-
.get(&context.component)
22+
.get(&context.package_id)
2323
.with_context(|| format!("No macros found in scope: {context:?}"))?;
2424

2525
let instance = plugin

scarb/tests/proc_macro_prebuilt.rs

+8-5
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use scarb_proc_macro_server_types::methods::expand::{ExpandInline, ExpandInlineM
88
use scarb_proc_macro_server_types::scope::ProcMacroScope;
99
use scarb_test_support::cairo_plugin_project_builder::CairoPluginProjectBuilder;
1010
use scarb_test_support::command::Scarb;
11-
use scarb_test_support::proc_macro_server::ProcMacroClient;
11+
use scarb_test_support::proc_macro_server::{DefinedMacrosInfo, ProcMacroClient};
1212
use scarb_test_support::project_builder::ProjectBuilder;
1313
use scarb_test_support::workspace_builder::WorkspaceBuilder;
1414
use snapbox::cmd::Command;
@@ -218,13 +218,16 @@ fn load_prebuilt_proc_macros() {
218218

219219
let mut proc_macro_client = ProcMacroClient::new_without_cargo(&project);
220220

221-
let component = proc_macro_client
222-
.defined_macros_for_package("test_package")
223-
.component;
221+
let DefinedMacrosInfo {
222+
package_id: compilation_unit_main_component_id,
223+
..
224+
} = proc_macro_client.defined_macros_for_package("test_package");
224225

225226
let response = proc_macro_client
226227
.request_and_wait::<ExpandInline>(ExpandInlineMacroParams {
227-
context: ProcMacroScope { component },
228+
context: ProcMacroScope {
229+
package_id: compilation_unit_main_component_id,
230+
},
228231
name: "some".to_string(),
229232
args: TokenStream::new("42".to_string()),
230233
})

scarb/tests/proc_macro_server.rs

+12-13
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use scarb_proc_macro_server_types::methods::expand::ExpandInline;
99
use scarb_proc_macro_server_types::methods::expand::ExpandInlineMacroParams;
1010
use scarb_proc_macro_server_types::scope::ProcMacroScope;
1111
use scarb_test_support::cairo_plugin_project_builder::CairoPluginProjectBuilder;
12+
use scarb_test_support::proc_macro_server::DefinedMacrosInfo;
1213
use scarb_test_support::proc_macro_server::ProcMacroClient;
1314
use scarb_test_support::proc_macro_server::SIMPLE_MACROS;
1415
use scarb_test_support::project_builder::ProjectBuilder;
@@ -33,7 +34,8 @@ fn defined_macros() {
3334

3435
let mut proc_macro_client = ProcMacroClient::new(&project);
3536

36-
let defined_macros = proc_macro_client.defined_macros_for_package("test_package");
37+
let DefinedMacrosInfo { defined_macros, .. } =
38+
proc_macro_client.defined_macros_for_package("test_package");
3739

3840
assert_eq!(&defined_macros.attributes, &["some".to_string()]);
3941
assert_eq!(&defined_macros.derives, &["some_derive".to_string()]);
@@ -78,13 +80,12 @@ fn expand_attribute() {
7880

7981
let mut proc_macro_client = ProcMacroClient::new(&project);
8082

81-
let component = proc_macro_client
82-
.defined_macros_for_package("test_package")
83-
.component;
83+
let DefinedMacrosInfo { package_id, .. } =
84+
proc_macro_client.defined_macros_for_package("test_package");
8485

8586
let response = proc_macro_client
8687
.request_and_wait::<ExpandAttribute>(ExpandAttributeParams {
87-
context: ProcMacroScope { component },
88+
context: ProcMacroScope { package_id },
8889
attr: "rename_to_very_new_name".to_string(),
8990
args: TokenStream::empty(),
9091
item: TokenStream::new("fn some_test_fn(){}".to_string()),
@@ -118,15 +119,14 @@ fn expand_derive() {
118119

119120
let mut proc_macro_client = ProcMacroClient::new(&project);
120121

121-
let component = proc_macro_client
122-
.defined_macros_for_package("test_package")
123-
.component;
122+
let DefinedMacrosInfo { package_id, .. } =
123+
proc_macro_client.defined_macros_for_package("test_package");
124124

125125
let item = TokenStream::new("fn some_test_fn(){}".to_string());
126126

127127
let response = proc_macro_client
128128
.request_and_wait::<ExpandDerive>(ExpandDeriveParams {
129-
context: ProcMacroScope { component },
129+
context: ProcMacroScope { package_id },
130130
derives: vec!["some_derive".to_string()],
131131
item,
132132
})
@@ -166,13 +166,12 @@ fn expand_inline() {
166166

167167
let mut proc_macro_client = ProcMacroClient::new(&project);
168168

169-
let component = proc_macro_client
170-
.defined_macros_for_package("test_package")
171-
.component;
169+
let DefinedMacrosInfo { package_id, .. } =
170+
proc_macro_client.defined_macros_for_package("test_package");
172171

173172
let response = proc_macro_client
174173
.request_and_wait::<ExpandInline>(ExpandInlineMacroParams {
175-
context: ProcMacroScope { component },
174+
context: ProcMacroScope { package_id },
176175
name: "replace_all_15_with_25".to_string(),
177176
args: TokenStream::new(
178177
"struct A { field: 15 , other_field: macro_call!(12)}".to_string(),

utils/scarb-proc-macro-server-types/src/methods/defined_macros.rs

+14-17
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,28 @@
1-
use crate::scope::CompilationUnitComponent;
1+
use std::collections::HashMap;
22

33
use super::Method;
44
use serde::{Deserialize, Serialize};
55

6-
/// Response structure containing a listed information
7-
/// about the macros used by packages from the workspace.
6+
/// Response structure containing a mapping from package IDs
7+
/// to the information about the macros they use.
88
///
99
/// # Invariant
10-
/// Each [`CompilationUnitComponentMacros`] in `macros_for_packages` should have
11-
/// a unique `component` field which identifies it in the response.
12-
/// Effectively, it simulates a HashMap which cannot be used directly
13-
/// because of the JSON serialization.
10+
/// Correct usage of this struct during proc macro server <-> LS communication
11+
/// relies on the implicit contract that keys of `macros_by_package_id` are of form
12+
/// `PackageId.to_serialized_string()` which is always equal to
13+
/// `scarb_metadata::CompilationUnitComponentId.repr`.
1414
#[derive(Clone, Debug, Default, Serialize, Deserialize)]
1515
pub struct DefinedMacrosResponse {
16-
/// A list of [`CompilationUnitComponentMacros`], describing macros
17-
/// available for each package from the workspace.
18-
pub macros_for_cu_components: Vec<CompilationUnitComponentMacros>,
16+
/// A mapping of the form: `package (as a serialized `PackageId`) -> macros info`.
17+
/// Contains serialized IDs of all packages from the workspace,
18+
/// mapped to the [`PackageDefinedMacrosInfo`], describing macros available for them.
19+
pub macros_by_package_id: HashMap<String, PackageDefinedMacrosInfo>,
1920
}
2021

21-
/// Response structure containing lists of all defined macros available for one compilation unit component.
22-
/// Provides the types of macros that can be expanded, such as attributes, inline macros, and derives.
22+
/// Response structure containing lists of all defined macros available for one package.
23+
/// Details the types of macros that can be expanded, such as attributes, inline macros, and derives.
2324
#[derive(Clone, Debug, Default, Serialize, Deserialize)]
24-
pub struct CompilationUnitComponentMacros {
25-
/// A component for which the macros are defined.
26-
/// It should identify [`CompilationUnitComponentMacros`]
27-
/// uniquely in the [`DefinedMacrosResponse`].
28-
pub component: CompilationUnitComponent,
25+
pub struct PackageDefinedMacrosInfo {
2926
/// List of attribute macro names available.
3027
pub attributes: Vec<String>,
3128
/// List of inline macro names available.
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,8 @@
11
use serde::{Deserialize, Serialize};
22

3-
/// Representation of the Scarb package.
4-
///
5-
/// # Invariants
6-
/// 1. (`name`, `discriminator`) pair must represent the package uniquely in the workspace.
7-
/// 2. `name` and `discriminator` must refer to the same `CompilationUnitComponent` and must be identical to those from `scarb-metadata`.
8-
/// At the moment, they are obtained using `CompilationUnitComponent::cairo_package_name`
9-
/// and `CompilationUnitComponentId::to_discriminator`, respectively.
10-
#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize, Hash)]
11-
pub struct CompilationUnitComponent {
12-
/// Name of the `CompilationUnitComponent` associated with the package.
13-
pub name: String,
14-
/// A `CompilationUnitComponent` discriminator.
15-
/// `None` only for corelib.
16-
pub discriminator: Option<String>,
17-
}
18-
19-
impl CompilationUnitComponent {
20-
/// Builds a new [`CompilationUnitComponent`] from `name` and `discriminator`
21-
/// without checking for consistency between them and with the metadata.
22-
///
23-
/// # Safety
24-
/// Communication between PMS and LS relies on the invariant that `name` and `discriminator`
25-
/// refer to the same CU component and are consistent with `scarb-metadata`.
26-
/// The caller must ensure correctness of the provided values.
27-
pub fn new(name: impl AsRef<str>, discriminator: impl AsRef<str>) -> Self {
28-
Self {
29-
name: name.as_ref().to_string(),
30-
discriminator: Some(discriminator.as_ref().to_string()),
31-
}
32-
}
33-
}
34-
353
/// A description of the location in the workspace where particular macro is available.
364
#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize, Hash)]
375
pub struct ProcMacroScope {
38-
/// A [`CompilationUnitComponent`] in which context the action occurs.
39-
pub component: CompilationUnitComponent,
6+
/// Serialized `PackageId` of the package in which context the action occurs.
7+
pub package_id: String,
408
}

0 commit comments

Comments
 (0)