Enforce decorating prod classes with final, abstract, or __Sealed

slackhq · May 21, 2024 · 5d63ad7 · 5d63ad7
1 parent 3c222a8
commit 5d63ad7
Show file tree

Hide file tree

Showing 6 changed files with 93 additions and 3 deletions.
diff --git a/src/analyzer/classlike_analyzer.rs b/src/analyzer/classlike_analyzer.rs
@@ -96,6 +96,53 @@ impl<'a> ClassLikeAnalyzer<'a> {
             analysis_data.issue_filter = Some(issue_filter.clone());
         }
 
+        if stmt.kind.is_cclass()
+            && classlike_storage
+                .direct_parent_class
+                .map(|parent_class_name| {
+                    codebase
+                        .classlike_infos
+                        .get(&parent_class_name)
+                        .map(|parent_classlike_storage| parent_classlike_storage.is_final)
+                        .unwrap_or(false)
+                })
+                .unwrap_or(false)
+        {
+            analysis_data.maybe_add_issue(
+                Issue::new(
+                    IssueKind::ExtendFinalClass,
+                    "Cannot extend final class".to_string(),
+                    classlike_storage.name_location,
+                    &Some(FunctionLikeIdentifier::Function(name)),
+                ),
+                statements_analyzer.get_config(),
+                statements_analyzer.get_file_path_actual(),
+            );
+        }
+
+        if stmt.kind.is_cclass()
+            && !stmt.is_xhp
+            && !classlike_storage.is_abstract
+            && !classlike_storage.is_final
+            && classlike_storage.child_classlikes.is_none()
+            && function_context.is_production(codebase)
+            && !classlike_storage
+                .all_parent_classes
+                .iter()
+                .any(|c| c == &StrId::EXCEPTION)
+        {
+            analysis_data.maybe_add_issue(
+                Issue::new(
+                    IssueKind::MissingFinalOrAbstract,
+                    "Class should always be declared abstract, final, or <<__Sealed>>".to_string(),
+                    classlike_storage.name_location,
+                    &Some(FunctionLikeIdentifier::Function(name)),
+                ),
+                statements_analyzer.get_config(),
+                statements_analyzer.get_file_path_actual(),
+            );
+        }
+
         let mut existing_enum_str_values = FxHashMap::default();
         let mut existing_enum_int_values = FxHashMap::default();
 

diff --git a/src/analyzer/expr/call/new_analyzer.rs b/src/analyzer/expr/call/new_analyzer.rs
@@ -301,8 +301,20 @@ fn analyze_named_constructor(
         // todo check for unsafe instantiation
     }
 
-    if storage.is_abstract && !can_extend {
-        // todo complain about abstract instantiation
+    if storage.is_abstract && !can_extend && !from_classname {
+        analysis_data.maybe_add_issue(
+            Issue::new(
+                IssueKind::AbstractInstantiation,
+                format!(
+                    "Cannot call new on abstract class {}",
+                    statements_analyzer.get_interner().lookup(&classlike_name)
+                ),
+                statements_analyzer.get_hpos(pos),
+                &context.function_context.calling_functionlike_id,
+            ),
+            statements_analyzer.get_config(),
+            statements_analyzer.get_file_path_actual(),
+        );
     }
 
     if storage.is_deprecated

diff --git a/src/code_info/function_context/mod.rs b/src/code_info/function_context/mod.rs
@@ -62,7 +62,17 @@ impl FunctionContext {
                     .unwrap()
                     .is_production_code
             }
-            _ => false,
+            _ => {
+                if let Some(calling_class) = self.calling_class {
+                    codebase
+                        .classlike_infos
+                        .get(&calling_class)
+                        .unwrap()
+                        .is_production_code
+                } else {
+                    false
+                }
+            }
         }
     }
 }
diff --git a/src/code_info/issue.rs b/src/code_info/issue.rs
@@ -14,12 +14,15 @@ use crate::{
 
 #[derive(Clone, PartialEq, Eq, Hash, Display, Debug, Serialize, Deserialize, EnumString)]
 pub enum IssueKind {
+    AbstractInstantiation,
+    ExtendFinalClass,
     CannotInferGenericParam,
     CustomIssue(Box<String>),
     DuplicateEnumValue,
     EmptyBlock,
     FalsableReturnStatement,
     FalseArgument,
+    MissingFinalOrAbstract,
     ForLoopInvalidation,
     ImmutablePropertyWrite,
     ImpossibleArrayAssignment,
@@ -311,6 +314,8 @@ pub fn get_issue_from_comment(
         return Some(Ok(IssueKind::UnusedAssignment));
     } else if trimmed_text.starts_with("HHAST_FIXME[NoJoinInAsyncFunction]") {
         return Some(Ok(IssueKind::NoJoinInAsyncFunction));
+    } else if trimmed_text.starts_with("HHAST_FIXME[FinalOrAbstractClass]") {
+        return Some(Ok(IssueKind::MissingFinalOrAbstract));
     }
 
     None

diff --git a/tests/inference/Class/finalOrAbstractClass/input.hack b/tests/inference/Class/finalOrAbstractClass/input.hack
@@ -0,0 +1,15 @@
+final class A {}
+
+abstract class B {}
+
+interface C {}
+
+<<__Sealed()>>
+class D {}
+
+class E extends \Exception {}
+
+class F {}
+
+/* HHAST_FIXME[FinalOrAbstractClass] */
+class G {}
diff --git a/tests/inference/Class/finalOrAbstractClass/output.txt b/tests/inference/Class/finalOrAbstractClass/output.txt
@@ -0,0 +1 @@
+ERROR: FinalOrAbstractClass - input.hack:12:7