fix: Fail early on invalid class and file names (serverpod#3288)

Author: nielsenko

tools/serverpod_cli/lib/src/analyzer/code_analysis_collector.dart

@@ -4,7 +4,7 @@ import 'package:super_string/super_string.dart';
 abstract class CodeAnalysisCollector {
   List<SourceSpanException> get errors;
 
-  static bool containsSeverErrors(List<SourceSpanException> errors) {
+  static bool containsSevereErrors(List<SourceSpanException> errors) {
     return errors.where(
       (error) {
         return error is! SourceSpanSeverityException ||
@@ -14,7 +14,7 @@ abstract class CodeAnalysisCollector {
     ).isNotEmpty;
   }
 
-  bool get hasSeverErrors => containsSeverErrors(errors);
+  bool get hasSevereErrors => containsSevereErrors(errors);
 
   void addError(SourceSpanException error);
 

tools/serverpod_cli/lib/src/analyzer/models/stateful_analyzer.dart

@@ -14,8 +14,8 @@ class StatefulAnalyzer {
   final Map<String, _ModelState> _modelStates = {};
 
   /// Returns true if any of the models have severe errors.
-  bool get hasSeverErrors => _modelStates.values.any(
-        (state) => CodeAnalysisCollector.containsSeverErrors(state.errors),
+  bool get hasSevereErrors => _modelStates.values.any(
+        (state) => CodeAnalysisCollector.containsSevereErrors(state.errors),
       );
 
   Function(Uri, CodeGenerationCollector)? _onErrorsChangedNotifier;
@@ -38,7 +38,7 @@ class StatefulAnalyzer {
   List<SerializableModelDefinition> get _validProjectModels => _modelStates
       .values
       .where(
-          (state) => !CodeAnalysisCollector.containsSeverErrors(state.errors))
+          (state) => !CodeAnalysisCollector.containsSevereErrors(state.errors))
       .where((state) => state.source.moduleAlias == defaultModuleAlias)
       .map((state) => state.model)
       .whereType<SerializableModelDefinition>()
@@ -139,7 +139,7 @@ class StatefulAnalyzer {
         parsedModels,
       );
 
-      if (collector.hasSeverErrors) {
+      if (collector.hasSevereErrors) {
         state.errors = collector.errors;
       } else {
         state.errors = [];

tools/serverpod_cli/lib/src/analyzer/models/validation/model_validator.dart

@@ -51,6 +51,19 @@ List<SourceSpanSeverityException> validateDuplicateFileName(
 
   if (modelDefinition == null) return [];
 
+  if (const [
+    'client', // we are already generating files with these names
+    'protocol',
+    'endpoints',
+  ].contains(modelDefinition.fileName)) {
+    return [
+      SourceSpanSeverityException(
+        'The file name "${modelDefinition.fileName}" is reserved and cannot be used.',
+        documentContents.span,
+      )
+    ];
+  }
+
   if (parsedModels.isFilePathUnique(modelDefinition)) {
     return [];
   }

tools/serverpod_cli/lib/src/analyzer/models/validation/restrictions.dart

@@ -156,7 +156,16 @@ class Restrictions {
       ];
     }
 
-    var reservedClassNames = const {'List', 'Set', 'Map', 'String', 'DateTime'};
+    const reservedClassNames = {
+      'List',
+      'Set',
+      'Map',
+      'String',
+      'DateTime',
+      'Client',
+      'Endpoints',
+      'Protocol',
+    };
     if (reservedClassNames.contains(className)) {
       return [
         SourceSpanSeverityException(

tools/serverpod_cli/lib/src/generator/generator.dart

@@ -16,7 +16,7 @@ Future<bool> performGenerate({
   log.debug('Analyzing serializable models in the protocol directory.');
 
   var models = modelAnalyzer.validateAll();
-  success &= !modelAnalyzer.hasSeverErrors;
+  success &= !modelAnalyzer.hasSevereErrors;
 
   log.debug('Generating files for serializable models.');
 
@@ -34,7 +34,7 @@ Future<bool> performGenerate({
     changedFiles: generatedModelFiles.toSet(),
   );
 
-  success &= !endpointAnalyzerCollector.hasSeverErrors;
+  success &= !endpointAnalyzerCollector.hasSevereErrors;
   endpointAnalyzerCollector.printErrors();
 
   log.debug('Generating the protocol.');

tools/serverpod_cli/lib/src/migrations/generator.dart

@@ -69,7 +69,7 @@ class MigrationGenerator {
     var modelDefinitions = StatefulAnalyzer(config, models, (uri, collector) {
       collector.printErrors();
 
-      if (collector.hasSeverErrors) {
+      if (collector.hasSevereErrors) {
         throw GenerateMigrationDatabaseDefinitionException();
       }
     }).validateAll();

tools/serverpod_cli/test/analyzer/models/stateful_analyzer/model_validation/file_name_conflicts_test.dart

@@ -91,4 +91,37 @@ void main() {
       );
     });
   });
+
+  for (var fileName in ['client', 'endpoints', 'protocol']) {
+    group(
+        'Given a model with the reserved file name "$fileName" when analyzing',
+        () {
+      late final modelSources = [
+        ModelSourceBuilder().withFileName(fileName).withYaml(
+          '''
+        class: Whatever
+        fields:
+          name: String
+        ''',
+        ).build(),
+      ];
+
+      late final collector = CodeGenerationCollector();
+
+      setUp(() {
+        StatefulAnalyzer(config, modelSources, onErrorsCollector(collector))
+            .validateAll();
+      });
+
+      test(
+          'then an error informs the user that there is a generated file collision.',
+          () {
+        var error = collector.errors.first;
+        expect(
+          error.message,
+          'The file name "$fileName" is reserved and cannot be used.',
+        );
+      });
+    });
+  }
 }

tools/serverpod_cli/test/analyzer/models/stateful_analyzer/model_validation/model_types_test.dart

@@ -293,6 +293,130 @@ void main() {
     },
   );
 
+  test(
+    'Given a class name with reserved value Endpoints, then give an error that the class name is reserved.',
+    () {
+      var models = [
+        ModelSourceBuilder().withYaml(
+          '''
+          class: Endpoints
+          fields:
+            name: String
+          ''',
+        ).build()
+      ];
+
+      var collector = CodeGenerationCollector();
+      StatefulAnalyzer(config, models, onErrorsCollector(collector))
+          .validateAll();
+
+      expect(
+        collector.errors,
+        isNotEmpty,
+        reason: 'Expected an error but none was generated.',
+      );
+
+      var error = collector.errors.first;
+      expect(
+        error.message,
+        'The class name "Endpoints" is reserved and cannot be used.',
+      );
+    },
+  );
+
+  test(
+    'Given a class name with reserved value DateTime, then give an error that the class name is reserved.',
+    () {
+      var models = [
+        ModelSourceBuilder().withYaml(
+          '''
+          class: DateTime
+          fields:
+            name: String
+          ''',
+        ).build()
+      ];
+
+      var collector = CodeGenerationCollector();
+      StatefulAnalyzer(config, models, onErrorsCollector(collector))
+          .validateAll();
+
+      expect(
+        collector.errors,
+        isNotEmpty,
+        reason: 'Expected an error but none was generated.',
+      );
+
+      var error = collector.errors.first;
+      expect(
+        error.message,
+        'The class name "DateTime" is reserved and cannot be used.',
+      );
+    },
+  );
+
+  test(
+    'Given a class name with reserved value Protocol, then give an error that the class name is reserved.',
+    () {
+      var models = [
+        ModelSourceBuilder().withYaml(
+          '''
+          class: Protocol
+          fields:
+            name: String
+          ''',
+        ).build()
+      ];
+
+      var collector = CodeGenerationCollector();
+      StatefulAnalyzer(config, models, onErrorsCollector(collector))
+          .validateAll();
+
+      expect(
+        collector.errors,
+        isNotEmpty,
+        reason: 'Expected an error but none was generated.',
+      );
+
+      var error = collector.errors.first;
+      expect(
+        error.message,
+        'The class name "Protocol" is reserved and cannot be used.',
+      );
+    },
+  );
+
+  test(
+    'Given a class name with reserved value Client, then give an error that the class name is reserved.',
+    () {
+      var models = [
+        ModelSourceBuilder().withYaml(
+          '''
+          class: Client
+          fields:
+            name: String
+          ''',
+        ).build()
+      ];
+
+      var collector = CodeGenerationCollector();
+      StatefulAnalyzer(config, models, onErrorsCollector(collector))
+          .validateAll();
+
+      expect(
+        collector.errors,
+        isNotEmpty,
+        reason: 'Expected an error but none was generated.',
+      );
+
+      var error = collector.errors.first;
+      expect(
+        error.message,
+        'The class name "Client" is reserved and cannot be used.',
+      );
+    },
+  );
+
   group('Given a model without any defined model type', () {
     test(
       'Then return a human readable error message informing the user that the model type is missing.',

tools/serverpod_cli/test/analyzer/models/stateful_analyzer/stateful_analyzer_test.dart

@@ -123,7 +123,7 @@ fields:
   });
 
   test(
-      'Given a model with a severe error (invalid syntax), when validating all, then hasSeverErrors returns true',
+      'Given a model with a severe error (invalid syntax), when validating all, then hasSevereErrors returns true',
       () {
     var yamlSource = ModelSourceBuilder().withYaml('''''').build();
 
@@ -133,7 +133,7 @@ fields:
     );
 
     statefulAnalyzer.validateAll();
-    expect(statefulAnalyzer.hasSeverErrors, true);
+    expect(statefulAnalyzer.hasSevereErrors, true);
   });
 
   test(

tools/serverpod_cli/test/util/string_manipulation_test.dart

@@ -215,7 +215,7 @@ void main() {
     });
 
     test(
-        'with a single excaped single quote when splitting then it is recognized as a single token.',
+        'with a single escaped single quote when splitting then it is recognized as a single token.',
         () {
       var result = splitIgnoringBracketsAndQuotes(
         'controlToken, "This \\\'is a default value", controlToken',
@@ -228,7 +228,7 @@ void main() {
     });
 
     test(
-        'with a single excaped double quote when splitting then it is recognized as a single token.',
+        'with a single escaped double quote when splitting then it is recognized as a single token.',
         () {
       var result = splitIgnoringBracketsAndQuotes(
         'controlToken, "This \\"is a default value", controlToken',