Fix issues with new inference, enable it by default

analysis_options.yaml

@@ -7,6 +7,7 @@ analyzer:
     unused_local_variable: error
     dead_code: error
     public_member_api_docs: ignore # turned on by user-facing subpackages
+    deprecated_member_use_from_same_package: ignore
   exclude:
     - "**/*.g.dart"
     # Will be analyzed anyway, nobody knows why ¯\_(ツ)_/¯. We're only analyzing lib/ and test/ as a workaround

docs/content/en/docs/Advanced Features/builder_options.md

@@ -52,19 +52,14 @@ At the moment, moor supports these options:
   column constraint (e.g. `user_name VARCHAR NOT NULL JSON KEY userName`)
 * `generate_connect_constructor`: Generate necessary code to support the [isolate runtime]({{< relref "isolates.md" >}}).
   This is a build option because isolates are still experimental. This will be the default option eventually.
-* `use_experimental_inference`: Use a new, experimental type inference algorithm when analyzing sql statements. The 
-  algorithm is designed to yield more accurate results on nullability and complex constructs. Note that it's in a 
-  preview state at the moment, which means that generated code might change after a minor update.
 * `sqlite_modules`: This list can be used to enable sqlite extensions, like those for json or full-text search.
   Modules have to be enabled explicitly because they're not supported on all platforms. See the following section for
   details.
-* `use_experimental_inference`: Enables a new type inference algorithm for sql statements.
-  The new algorithm is much better at handling complex statements and nullability. 
-  However, it's still in development and may not work in all cases yet. Please report any issues you can find.
-  __Warning:__ The new type inference algorithm is in development and does not obey to semantic versioning.
-  Results and generated code might change in moor versions not declared as breaking.
 * `eagerly_load_dart_ast`: Moor's builder will load the resolved AST whenever it encounters a Dart file,
   instead of lazily when it reads a table. This is used to investigate rare builder crashes. 
+* `legacy_type_inference`: Use the old type inference from moor 1 and 2. Note that `use_experimental_inference`
+   is now the default and no longer exists.
+   If you're using this flag, please open an issue and explain how the new inference isn't working for you, thanks!
 
 ## Available extensions
 

moor/build.yaml

@@ -8,7 +8,6 @@ targets:
           generate_connect_constructor: true
           compact_query_methods: true
           write_from_json_string_constructor: true
-          use_experimental_inference: true
           eagerly_load_dart_ast: true
           sqlite_modules:
             - json1

moor_generator/lib/src/analyzer/options.dart

@@ -54,11 +54,10 @@ class MoorOptions {
   @JsonKey(name: 'generate_connect_constructor', defaultValue: false)
   final bool generateConnectConstructor;
 
-  /// Whether the new, experimental type inference engine should be used.
-  ///
-  /// The new engine is experimental at the moment.
-  @JsonKey(name: 'use_experimental_inference', defaultValue: false)
-  final bool useExperimentalInference;
+  /// Whether the old, legacy type inference engine should be used when
+  /// analyzing custom sql queries.
+  @JsonKey(name: 'legacy_type_inference', defaultValue: false)
+  final bool legacyTypeInference;
 
   @JsonKey(name: 'sqlite_modules', defaultValue: [])
   final List<SqlModule> modules;
@@ -77,7 +76,7 @@ class MoorOptions {
       this.useDataClassNameForCompanions = false,
       this.useColumnNameAsJsonKeyWhenDefinedInMoorFile = false,
       this.generateConnectConstructor = false,
-      this.useExperimentalInference = false,
+      this.legacyTypeInference = false,
       this.eagerlyLoadDartAst = false,
       this.modules = const []});
 

moor_generator/lib/src/analyzer/options.g.dart

@@ -16,7 +16,7 @@ MoorOptions _$MoorOptionsFromJson(Map<String, dynamic> json) {
       'use_data_class_name_for_companions',
       'use_column_name_as_json_key_when_defined_in_moor_file',
       'generate_connect_constructor',
-      'use_experimental_inference',
+      'legacy_type_inference',
       'sqlite_modules',
       'eagerly_load_dart_ast'
     ]);
@@ -44,8 +44,8 @@ MoorOptions _$MoorOptionsFromJson(Map<String, dynamic> json) {
       generateConnectConstructor: $checkedConvert(
               json, 'generate_connect_constructor', (v) => v as bool) ??
           false,
-      useExperimentalInference: $checkedConvert(
-              json, 'use_experimental_inference', (v) => v as bool) ??
+      legacyTypeInference:
+          $checkedConvert(json, 'legacy_type_inference', (v) => v as bool) ??
               false,
       eagerlyLoadDartAst:
           $checkedConvert(json, 'eagerly_load_dart_ast', (v) => v as bool) ??

moor_generator/lib/src/analyzer/session.dart

@@ -44,7 +44,7 @@ class MoorSession {
         if (options.hasModule(SqlModule.json1)) const Json1Extension(),
         if (options.hasModule(SqlModule.moor_ffi)) const MoorFfiExtension(),
       ],
-      enableExperimentalTypeInference: options.useExperimentalInference,
+      useLegacyTypeInference: options.legacyTypeInference,
     );
 
     return SqlEngine(sqlOptions);

moor_generator/test/analyzer/moor/moor_ffi_extension_test.dart

@@ -89,10 +89,7 @@ import 'a.moor';
 wrongArgs: SELECT sin(oid, foo) FROM numbers;
         '''
       },
-      options: const MoorOptions(
-        useExperimentalInference: true,
-        modules: [SqlModule.moor_ffi],
-      ),
+      options: const MoorOptions(modules: [SqlModule.moor_ffi]),
     );
 
     final fileA = await state.analyze('package:foo/a.moor');

moor_generator/test/analyzer/sql_queries/inference_test.dart

@@ -3,7 +3,7 @@ import 'package:moor_generator/src/analyzer/options.dart';
 import 'package:moor_generator/src/analyzer/runner/results.dart';
 import 'package:test/test.dart';
 
-import 'utils.dart';
+import '../utils.dart';
 
 void main() {
   test('experimental inference - integration test', () async {
@@ -35,7 +35,7 @@ totalDurationByArtist:
     INNER JOIN tracks ON tracks.album = albums.id
   GROUP BY artists.id;
     '''
-    }, options: const MoorOptions(useExperimentalInference: true));
+    }, options: const MoorOptions());
 
     final file = await state.analyze('package:foo/a.moor');
     final result = file.currentResult as ParsedMoorFile;

sqlparser/CHANGELOG.md

@@ -5,6 +5,8 @@
   tables with a comma will now be parsed as a `JoinClause`.
 - Changed `SelectStatementAsSource.statement` from `SelectStatement` to `BaseSelectStatement` and allow
   compound select statements to appear in a `FROM` clause
+- The new type inference engine is now enabled by default and the `enableExperimentalTypeInference` option
+  has been removed. To continue using the old engine, the `useLegacyTypeInference` flag can be used.
 
 ## 0.7.0
 

sqlparser/lib/src/analysis/context.dart

@@ -29,6 +29,7 @@ class AnalysisContext {
   /// A resolver that can be used to obtain the type of a [Typeable]. This
   /// mostly applies to [Expression]s, [Reference]s, [Variable]s and
   /// [ResultSet.resolvedColumns] of a select statement.
+  @Deprecated('Used for legacy types only - consider migrating to types2')
   /* late final */ TypeResolver types;
 
   /// Experimental new type resolver with better support for nullability and
@@ -38,18 +39,13 @@ class AnalysisContext {
   /// a [Variable] and [ResultSet.resolvedColumns] may be resolved or inferred.
   ///
   /// This field is null when experimental type inference is disabled.
-  ///
-  /// Please note that types2 is experimental at the moment. Changes to how
-  /// [types] resolves types are considered breaking and are handled
-  /// accordingly. [types2] may change results in any update.
-  @experimental
   TypeInferenceResults types2;
 
   /// Constructs a new analysis context from the AST and the source sql.
   AnalysisContext(this.root, this.sql, this.engineOptions,
       {AnalyzeStatementOptions stmtOptions, this.schemaSupport})
       : stmtOptions = stmtOptions ?? const AnalyzeStatementOptions() {
-    if (!engineOptions.enableExperimentalTypeInference) {
+    if (engineOptions.useLegacyTypeInference) {
       types = TypeResolver(this, engineOptions);
     }
   }

sqlparser/lib/src/analysis/types2/graph/type_graph.dart

@@ -9,7 +9,7 @@ class TypeGraph {
   final List<TypeRelation> _relations = [];
 
   final Map<Typeable, List<TypeRelation>> _edges = {};
-  final Set<MultiSourceRelation> _multiSources = {};
+  final Set<Typeable> _candidateForLaxMultiPropagation = {};
   final List<DefaultType> _defaultTypes = [];
 
   ResolvedType operator [](Typeable t) {
@@ -51,14 +51,17 @@ class TypeGraph {
   void performResolve() {
     _indexRelations();
 
-    final queue = List.of(_knownTypes.keys);
+    var queue = List.of(_knownTypes.keys);
     while (queue.isNotEmpty) {
       _propagateTypeInfo(queue, queue.removeLast());
     }
 
-    // propagate many-to-one sources where we don't know each source type
-    for (final remaining in _multiSources) {
-      _propagateManyToOne(remaining, queue);
+    // propagate many-to-one sources where we don't know each source type, but
+    // some of them.
+    queue = List.of(_candidateForLaxMultiPropagation);
+    while (queue.isNotEmpty) {
+      _propagateTypeInfo(queue, queue.removeLast(),
+          laxMultiSourcePropagation: true);
     }
 
     // apply default types
@@ -77,7 +80,8 @@ class TypeGraph {
     }
   }
 
-  void _propagateTypeInfo(List<Typeable> resolved, Typeable t) {
+  void _propagateTypeInfo(List<Typeable> resolved, Typeable t,
+      {bool laxMultiSourcePropagation = false}) {
     if (!_edges.containsKey(t)) return;
 
     // propagate changes
@@ -93,10 +97,14 @@ class TypeGraph {
       } else if (edge is CopyAndCast) {
         _copyType(resolved, t, edge.target, this[t].cast(edge.cast));
       } else if (edge is MultiSourceRelation) {
-        // handle many-to-one changes, if all targets have been resolved
-        if (edge.from.every(knowsType)) {
-          _multiSources.remove(edge);
+        // handle many-to-one changes, if all targets have been resolved or
+        // lax handling is enabled.
+        if (laxMultiSourcePropagation || edge.from.every(knowsType)) {
           _propagateManyToOne(edge, resolved);
+
+          _candidateForLaxMultiPropagation.removeAll(edge.from);
+        } else {
+          _candidateForLaxMultiPropagation.add(t);
         }
       }
     }
@@ -165,7 +173,6 @@ class TypeGraph {
     }
 
     void putAll(MultiSourceRelation r) {
-      _multiSources.add(r);
       for (final element in r.from) {
         put(element, r);
       }

sqlparser/lib/src/analysis/types2/resolving_visitor.dart

@@ -11,6 +11,8 @@ const _expectString = ExactTypeExpectation.laxly(_textType);
 class TypeResolver extends RecursiveVisitor<TypeExpectation, void> {
   final TypeInferenceSession session;
 
+  final Set<Column> _handledColumns = {};
+
   TypeResolver(this.session);
 
   void run(AstNode root) {
@@ -414,6 +416,10 @@ class TypeResolver extends RecursiveVisitor<TypeExpectation, void> {
     }
   }
 
+  FunctionHandler /*?*/ _functionHandlerFor(ExpressionInvocation e) {
+    return session.options.addedFunctions[e.name.toLowerCase()];
+  }
+
   ResolvedType _resolveInvocation(ExpressionInvocation e) {
     final params = e.expandParameters();
     void nullableIfChildIs() {
@@ -516,11 +522,9 @@ class TypeResolver extends RecursiveVisitor<TypeExpectation, void> {
         return null;
     }
 
-    final addedFunctions = session.options.addedFunctions;
-    if (addedFunctions.containsKey(lowercaseName)) {
-      return addedFunctions[lowercaseName]
-          .inferReturnType(session.context, e, params)
-          ?.type;
+    final extensionHandler = _functionHandlerFor(e);
+    if (extensionHandler != null) {
+      return extensionHandler.inferReturnType(session.context, e, params)?.type;
     }
 
     session.context.reportError(AnalysisError(
@@ -542,6 +546,25 @@ class TypeResolver extends RecursiveVisitor<TypeExpectation, void> {
       visited.add(secondParam);
     }
 
+    final extensionHandler =
+        e is ExpressionInvocation ? _functionHandlerFor(e) : null;
+    if (extensionHandler != null) {
+      for (final arg in params) {
+        if (arg is! Expression) continue;
+
+        final expressionArgument = arg as Expression;
+
+        final result = extensionHandler.inferArgumentType(
+            session.context, e, expressionArgument);
+        final type = result?.type;
+        if (type != null) {
+          session._markTypeResolved(expressionArgument, type);
+        }
+
+        visited.add(expressionArgument);
+      }
+    }
+
     return visited;
   }
 
@@ -558,12 +581,18 @@ class TypeResolver extends RecursiveVisitor<TypeExpectation, void> {
   }
 
   void _handleColumn(Column column) {
-    if (session.graph.knowsType(column)) return;
+    if (session.graph.knowsType(column) || _handledColumns.contains(column)) {
+      return;
+    }
+    _handledColumns.add(column);
 
     if (column is TableColumn) {
       session._markTypeResolved(column, column.type);
     } else if (column is ExpressionColumn) {
       _lazyCopy(column, column.expression);
+    } else if (column is CompoundSelectColumn) {
+      session._addRelation(CopyEncapsulating(column, column.columns));
+      column.columns.forEach(_handleColumn);
     } else if (column is DelegatedColumn && column.innerColumn != null) {
       _handleColumn(column.innerColumn);
       _lazyCopy(column, column.innerColumn);
@@ -605,5 +634,7 @@ class _ResultColumnVisitor extends RecursiveVisitor<void, void> {
     if (stmt.resolvedColumns != null) {
       stmt.resolvedColumns.forEach(resolver._handleColumn);
     }
+
+    visitChildren(stmt, arg);
   }
 }

sqlparser/lib/src/engine/options.dart

@@ -6,8 +6,10 @@ class EngineOptions {
   /// extensions enabled.
   final bool useMoorExtensions;
 
-  /// Enables the new, experimental type inference.
-  final bool enableExperimentalTypeInference;
+  /// Whether the old type inference algorithm should be used.
+  ///
+  /// Defaults to false.
+  final bool useLegacyTypeInference;
 
   /// All [Extension]s that have been enabled in this sql engine.
   final List<Extension> enabledExtensions;
@@ -24,7 +26,7 @@ class EngineOptions {
   EngineOptions({
     this.useMoorExtensions = false,
     this.enabledExtensions = const [],
-    this.enableExperimentalTypeInference = false,
+    this.useLegacyTypeInference = false,
   });
 
   void addFunctionHandler(FunctionHandler handler) {

sqlparser/lib/src/engine/sql_engine.dart

@@ -190,13 +190,13 @@ class SqlEngine {
         ..acceptWithoutArg(ColumnResolver(context))
         ..acceptWithoutArg(ReferenceResolver(context));
 
-      if (options.enableExperimentalTypeInference) {
+      if (options.useLegacyTypeInference) {
+        node.acceptWithoutArg(TypeResolvingVisitor(context));
+      } else {
         final session = t2.TypeInferenceSession(context, options);
         final resolver = t2.TypeResolver(session);
         resolver.run(node);
         context.types2 = session.results;
-      } else {
-        node.acceptWithoutArg(TypeResolvingVisitor(context));
       }
 
       node.acceptWithoutArg(LintingVisitor(options, context));

sqlparser/test/analysis/types/resolver_test.dart

@@ -42,7 +42,7 @@ Map<String, ResolveResult> _types = {
 void main() {
   _types.forEach((sql, resolvedType) {
     test('types: resolves in $sql', () {
-      final engine = SqlEngine()
+      final engine = SqlEngine(EngineOptions(useLegacyTypeInference: true))
         ..registerTable(demoTable)
         ..registerTable(anotherTable);
       final content = engine.analyze(sql);

sqlparser/test/analysis/types2/misc_cases_test.dart

@@ -39,9 +39,7 @@ const Map<String, ResolvedType> _types = {
 };
 
 SqlEngine _spawnEngine() {
-  return SqlEngine(EngineOptions(enableExperimentalTypeInference: true))
-    ..registerTable(demoTable)
-    ..registerTable(anotherTable);
+  return SqlEngine()..registerTable(demoTable)..registerTable(anotherTable);
 }
 
 void main() {

sqlparser/test/engine/module/json1_test.dart

@@ -2,18 +2,18 @@ import 'package:sqlparser/sqlparser.dart';
 import 'package:test/test.dart';
 
 void main() {
-  group('json with stable inference', () {
+  group('json with legacy inference', () {
     _runTests(false);
   });
 
-  group('json with types2', () {
+  group('json with new type inference', () {
     _runTests(true);
   });
 }
 
 void _runTests(bool types2) {
   final engine = SqlEngine(EngineOptions(
-    enableExperimentalTypeInference: types2,
+    useLegacyTypeInference: !types2,
     enabledExtensions: const [Json1Extension()],
   ));
   // add user (name, phone) table