From 685ab2b4262bf894dd708e89a7873c235e02b486 Mon Sep 17 00:00:00 2001 From: Aaron Orenstein Date: Mon, 13 Feb 2023 20:50:48 -0800 Subject: [PATCH] Property attributes Summary: Support for property attributes by emitting them as textual comments Reviewed By: davidpichardie Differential Revision: D42442725 fbshipit-source-id: b0e0ffb761e5ae8e7dffafe4c803ea0b78e6218c --- .../hack/src/hackc/ir/conversions/textual/class.rs | 52 +++++++++++++------ hphp/hack/src/hackc/ir/conversions/textual/lib.rs | 8 ++- .../src/hackc/ir/conversions/textual/textual.rs | 60 +++++++++++++++++++--- .../src/hackc/ir/conversions/textual/writer.rs | 2 +- hphp/hack/src/hackc/test/infer/class.hack | 3 +- hphp/hack/src/hackc/test/infer/class_tc.hack | 1 + 6 files changed, 100 insertions(+), 26 deletions(-) create mode 100644 hphp/hack/src/hackc/test/infer/class_tc.hack diff --git a/hphp/hack/src/hackc/ir/conversions/textual/class.rs b/hphp/hack/src/hackc/ir/conversions/textual/class.rs index 3ab35870758..3cf0a73347c 100644 --- a/hphp/hack/src/hackc/ir/conversions/textual/class.rs +++ b/hphp/hack/src/hackc/ir/conversions/textual/class.rs @@ -28,6 +28,7 @@ use crate::mangle::Mangle; use crate::mangle::MangleClass as _; use crate::mangle::MangleWithClass; use crate::state::UnitState; +use crate::textual::FieldAttribute; use crate::textual::TextualFile; use crate::typed_value; use crate::types::convert_ty; @@ -143,7 +144,7 @@ impl ClassState<'_, '_, '_> { metadata.insert("final", self.class.flags.is_final().into()); } - let mut fields: Vec<(String, textual::Ty, textual::Visibility)> = Vec::new(); + let mut fields: Vec> = Vec::new(); let properties = std::mem::take(&mut self.class.properties); for prop in &properties { if prop.flags.is_static() == is_static.as_bool() { @@ -165,18 +166,12 @@ impl ClassState<'_, '_, '_> { .map(|id| id.mangle_class(is_static, self.strings())) .collect_vec(); - let fields = fields.iter().map(|(name, ty, visibility)| textual::Field { - name: name.as_str(), - ty, - visibility: *visibility, - }); - let cname = self.class.name.mangle_class(is_static, self.strings()); self.txf.define_type( &cname, &self.class.src_loc, extends.iter().map(String::as_str), - fields, + fields.into_iter(), metadata.iter().map(|(k, v)| (*k, v)), )?; @@ -185,14 +180,14 @@ impl ClassState<'_, '_, '_> { fn write_property( &mut self, - fields: &mut Vec<(String, textual::Ty, textual::Visibility)>, + fields: &mut Vec>, prop: &ir::Property<'_>, ) -> Result { let ir::Property { name, mut flags, ref attributes, - visibility, + visibility: ir_visibility, ref initial_value, ref type_info, doc_comment: _, @@ -202,7 +197,7 @@ impl ClassState<'_, '_, '_> { let name = name.mangle(&self.unit_state.strings); - let vis = if flags.is_private() { + let visibility = if flags.is_private() { textual::Visibility::Private } else if flags.is_protected() { textual::Visibility::Protected @@ -214,14 +209,35 @@ impl ClassState<'_, '_, '_> { flags.clear(ir::Attr::AttrPublic); flags.clear(ir::Attr::AttrSystemInitialValue); + let mut tx_attributes = Vec::new(); + let comments = Vec::new(); + if !flags.is_empty() { trace!("CLASS FLAGS: {:?}", flags); textual_todo! { self.txf.write_comment(&format!("TODO: class flags: {flags:?}"))? }; } - if !attributes.is_empty() { - textual_todo! { self.txf.write_comment(&format!("TODO: class attributes: {attributes:?}"))? }; + + for attribute in attributes { + // We don't do anything with class attributes. They don't affect + // normal program flow - but can be seen by reflection so it's + // questionable if we need them for analysis. + let name = attribute + .name + .mangle_class(IsStatic::NonStatic, &self.unit_state.strings); + if attribute.arguments.is_empty() { + tx_attributes.push(FieldAttribute::Unparameterized { name }); + } else { + let mut parameters = Vec::new(); + for arg in &attribute.arguments { + textual_todo! { + parameters.push(format!("TODO: {arg:?}")); + } + } + tx_attributes.push(FieldAttribute::Parameterized { name, parameters }); + } } - if visibility == ir::Visibility::Private { + + if ir_visibility == ir::Visibility::Private { self.txf.write_comment(&format!("TODO: private {name}"))?; } if let Some(initial_value) = initial_value { @@ -231,7 +247,13 @@ impl ClassState<'_, '_, '_> { let ty = convert_ty(&type_info.enforced, &self.unit_state.strings); - fields.push((name, ty, vis)); + fields.push(textual::Field { + name: name.into(), + ty: ty.into(), + visibility, + attributes: tx_attributes, + comments, + }); Ok(()) } diff --git a/hphp/hack/src/hackc/ir/conversions/textual/lib.rs b/hphp/hack/src/hackc/ir/conversions/textual/lib.rs index 10ce6f51f2e..7b4f93873a8 100644 --- a/hphp/hack/src/hackc/ir/conversions/textual/lib.rs +++ b/hphp/hack/src/hackc/ir/conversions/textual/lib.rs @@ -14,6 +14,10 @@ pub static KEEP_GOING: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); +pub fn keep_going_mode() -> bool { + KEEP_GOING.load(std::sync::atomic::Ordering::Acquire) +} + // If KEEP_GOING is false then execute a 'todo!' otherwise call the function. // // Used like: @@ -21,14 +25,14 @@ pub static KEEP_GOING: std::sync::atomic::AtomicBool = std::sync::atomic::Atomic #[allow(unused)] macro_rules! textual_todo { ( message = ($($msg:expr),+), $($rest:tt)+ ) => { - (if $crate::KEEP_GOING.load(std::sync::atomic::Ordering::Acquire) { + (if $crate::keep_going_mode() { $($rest)+ } else { todo!($($msg),+) }) }; ( $($rest:tt)+ ) => { - (if $crate::KEEP_GOING.load(std::sync::atomic::Ordering::Acquire) { + (if $crate::keep_going_mode() { $($rest)+ } else { todo!() diff --git a/hphp/hack/src/hackc/ir/conversions/textual/textual.rs b/hphp/hack/src/hackc/ir/conversions/textual/textual.rs index 1303cefc681..695d6014c3f 100644 --- a/hphp/hack/src/hackc/ir/conversions/textual/textual.rs +++ b/hphp/hack/src/hackc/ir/conversions/textual/textual.rs @@ -7,6 +7,7 @@ //! basically no business logic and just provides a type-safe way to write //! Textual. +use std::borrow::Cow; use std::fmt; use std::sync::Arc; @@ -189,13 +190,37 @@ impl<'a> TextualFile<'a> { let mut sep = "\n"; for f in fields { + for comment in &f.comments { + writeln!(self.w, "{sep}{INDENT}// {comment}")?; + sep = ""; + } write!( self.w, - "{sep}{INDENT}{name}: {vis} {ty}", + "{sep}{INDENT}{name}: {vis} ", name = f.name, - ty = FmtTy(f.ty), vis = f.visibility.decl() )?; + + for attr in &f.attributes { + write!(self.w, ".{name} ", name = attr.name())?; + match attr { + FieldAttribute::Unparameterized { .. } => {} + FieldAttribute::Parameterized { + name: _, + parameters, + } => { + let mut i = parameters.iter(); + let param = i.next().unwrap(); + write!(self.w, "= \"{param}\"")?; + for param in i { + write!(self.w, ", \"{param}\"")?; + } + write!(self.w, " ")?; + } + } + } + + write!(self.w, "{ty}", ty = FmtTy(&f.ty))?; sep = ";\n"; } @@ -205,9 +230,9 @@ impl<'a> TextualFile<'a> { Ok(()) } - pub(crate) fn set_attribute(&mut self, attr: Attribute) -> Result { + pub(crate) fn set_attribute(&mut self, attr: FileAttribute) -> Result { match attr { - Attribute::SourceLanguage(lang) => { + FileAttribute::SourceLanguage(lang) => { writeln!(self.w, ".source_language = \"{lang}\"")?; } } @@ -725,7 +750,7 @@ where } } -pub(crate) enum Attribute { +pub(crate) enum FileAttribute { SourceLanguage(String), } @@ -978,8 +1003,29 @@ impl Visibility { } } +pub(crate) enum FieldAttribute { + Unparameterized { + name: String, + }, + #[allow(dead_code)] + Parameterized { + name: String, + parameters: Vec, + }, +} + +impl FieldAttribute { + fn name(&self) -> &str { + match self { + Self::Unparameterized { name } | Self::Parameterized { name, .. } => name, + } + } +} + pub(crate) struct Field<'a> { - pub name: &'a str, - pub ty: &'a Ty, + pub name: Cow<'a, str>, + pub ty: Cow<'a, Ty>, pub visibility: Visibility, + pub attributes: Vec, + pub comments: Vec, } diff --git a/hphp/hack/src/hackc/ir/conversions/textual/writer.rs b/hphp/hack/src/hackc/ir/conversions/textual/writer.rs index df8dc88f952..5d819b63a0f 100644 --- a/hphp/hack/src/hackc/ir/conversions/textual/writer.rs +++ b/hphp/hack/src/hackc/ir/conversions/textual/writer.rs @@ -31,7 +31,7 @@ pub fn textual_writer( let escaped_path = escaper::escape(path.display().to_string()); txf.write_comment(&format!("{UNIT_START_MARKER} {escaped_path}"))?; - txf.set_attribute(textual::Attribute::SourceLanguage("hack".to_string()))?; + txf.set_attribute(textual::FileAttribute::SourceLanguage("hack".to_string()))?; txf.debug_separator()?; let mut state = UnitState::new(Arc::clone(&unit.strings)); diff --git a/hphp/hack/src/hackc/test/infer/class.hack b/hphp/hack/src/hackc/test/infer/class.hack index 698f3a992f0..57e6f97be95 100644 --- a/hphp/hack/src/hackc/test/infer/class.hack +++ b/hphp/hack/src/hackc/test/infer/class.hack @@ -3,7 +3,7 @@ // TEST-CHECK-BAL: type C$static // CHECK: type C$static = .kind="class" .static { // CHECK: prop3: .public *HackFloat; -// CHECK: prop4: .public *HackMixed +// CHECK: prop4: .public .SomeAttribute *HackMixed // CHECK: } // TEST-CHECK-BAL: "type C " @@ -27,6 +27,7 @@ class C { public int $prop1 = 42; public string $prop2 = "hello"; public static float $prop3 = 3.14; + <> public static mixed $prop4 = null; const int MY_CONSTANT = 7; diff --git a/hphp/hack/src/hackc/test/infer/class_tc.hack b/hphp/hack/src/hackc/test/infer/class_tc.hack new file mode 100644 index 00000000000..aa0d87ae806 --- /dev/null +++ b/hphp/hack/src/hackc/test/infer/class_tc.hack @@ -0,0 +1 @@ +interface SomeAttribute extends HH\StaticPropertyAttribute { } -- 2.11.4.GIT