Preventing getter name conflicts in step-by-step migrations

[jmatth]

Closes #2610.

I added a couple methods to Scope to keep track of which names have been used and allow the caller to specify how non-conflicting names should be calculated. As is this fixes the issue in my codebase with a table named entities.

In #2610 you mentioned:

This is not only a problem with table names, but also with the getter names for columns potentially colliding with columns inherited from VersionedTable (and its superclasses).

I began looking into addressing that as well but as far as I can tell any name collisions there will fail while generating Drift code for runtime. Both VersionedTable and table the table classes generated for runtime extend Table and mixin TableInfo.

Are there tests for the code generation I should be updating or adding to? I see there are tests for various parts of the drift_dev package but couldn't find any that covered SchemaVersionWriter.

[simolus3]

Thanks for the contribution, this looks great!

Are there tests for the code generation I should be updating or adding to? I see there are tests for various parts of the drift_dev package but couldn't find any that covered SchemaVersionWriter.

Yeah, at the moment I'm using the generated code in examples/migrations and examples/app as an integration test for the writer, there aren't any unit tests for it. This is probably a good opportunity to get started with that, maybe something like this can serve as a starting point:

import 'package:drift/drift.dart';
import 'package:drift_dev/src/analysis/options.dart';
import 'package:drift_dev/src/analysis/results/results.dart';
import 'package:drift_dev/src/writer/import_manager.dart';
import 'package:drift_dev/src/writer/schema_version_writer.dart';
import 'package:drift_dev/src/writer/writer.dart';
import 'package:test/test.dart';

void main() {
  final fakeUri = Uri.parse('drift:hidden');

  test('avoids conflict with getters in schema class', () async {
    final imports = LibraryImportManager();
    final writer = Writer(
      const DriftOptions.defaults(),
      generationOptions: GenerationOptions(imports: imports),
    );
    imports.linkToWriter(writer);

    final table = DriftTable(
      DriftElementId(fakeUri, 'database'),
      DriftDeclaration(fakeUri, -1, ''),
      columns: [
        DriftColumn(
          sqlType: DriftSqlType.int,
          nullable: false,
          nameInSql: 'foo',
          nameInDart: 'foo',
          declaration: DriftDeclaration(fakeUri, -1, ''),
        ),
      ],
      baseDartName: 'database',
      nameOfRowClass: 'MyTableData',
    );

    SchemaVersionWriter(
      [
        SchemaVersion(
          1,
          [table],
          const {},
        ),
        SchemaVersion(
          2,
          [table],
          const {},
        ),
      ],
      writer.child(),
    ).write();

    expect(writer.writeGenerated(), contains('...'));
  });
}

[simolus3]

I added a couple methods to Scope to keep track of which names have been used and allow the caller to specify how non-conflicting names should be calculated

Nice idea!

[simolus3]

The implementation looks good to me, I just have two small comments on the test.

[simolus3]

Let's remove this debug print.

[simolus3]

Thanks for the contribution!