Escape SQL column names that conflict with table class members

[efim0v]

Fixes #2100

Tables defined in SQL are generated as classes that extend Table and mix in TableInfo (or ResultSet with drift3_preview), with a field for each column. This applies to .drift files and to the tables drift reconstructs from SQL, e.g. for schema dump on a database file or for schema snapshots that contain fixed_sql. The Dart name of a column was derived from its SQL name without looking at the members of those classes, so a column like text, integer, table_name, primary_key, map or hash_code generated code that doesn't compile:

Error: Can't declare a member that conflicts with an inherited one.
late final i2.GeneratedColumn<String> text = i2.GeneratedColumn<String>('text', aliasedName, true, ...

Fix

Like #3838 did for reserved keywords, this appends a $ to such names. dartNameForSqlColumn gets an optional reservedNames parameter, and the drift table resolver passes the names of the members of the generated table class: those of Object, Table and TableInfo (Table and ResultSet for drift3), plus moduleAndArgs for virtual tables. CREATE TABLE foo (text TEXT) now generates a text$ column getter, while the SQL name and the default JSON key stay text.

Since every reserved name is a member of the generated class, a column with one of these names never compiled before, so columns that work today keep their names (with one drift3 exception in the schema tools, see the end). For example, alias and columns are only reserved with drift3, whose table classes have members with those names. Names set explicitly with AS are left alone, and views and query result classes (which don't extend Table) are not affected.

I also updated the sentence in the drift files docs that suggested AS for this case, and added a changelog entry.

Tests

• test/analysis/resolver/drift/table_test.dart: checks the derived names for drift 2 (including an fts5 table with a module_and_args column) and for drift3, including names that are kept because they aren't members (alias for drift 2, map for drift3).
• test/services/schema/writer_test.dart: for drift 2 and drift3, reads the members of Table and TableInfo/ResultSet from package:drift with the analyzer, defines a table with one column per member and compiles the generated table class in the schema isolate (createSchemaJson(throwOnSchemaIsolateFailure: true)). On develop this fails with 'attachedDatabase' is already declared in this scope (drift 2) and 'alias' is already declared in this scope (drift3). Because the names come from the analyzer, the test will also fail if a member is added to these classes without updating the list.

In drift_dev, dart format -o none --set-exit-if-changed ., dart analyze --fatal-infos --fatal-warnings and dart test pass (512 passed, 1 skipped; I ran the tests with --timeout 4x because the machine was heavily loaded).

I also ran a full build_runner build of a throwaway .drift file with such columns and an fts5 table, generating data classes, companions and the manager. On develop, dart analyze reports errors such as Class 'Foo' can't define field 'text' and have method 'Table.text' with the same name in the generated file. With this change it reports no issues, and inserting and querying a row through the generated API works. With the drift3 preview enabled, the generated code for these columns analyzes cleanly as well.

Not covered here, so I left these out of this change:

• With drift3, a column named map still breaks the toColumns method of the generated data class, which declares a local variable called map. That isn't related to inherited members.
• The schema tools analyze the SQL they rebuild tables from (schema dump on a database file, snapshots with fixed_sql) with default options rather than the project's, so with drift3 those tables are checked against the drift 2 members. A column named alias still conflicts there, and names that are only drift 2 members (like map or attached_database) get a $. Fixing that needs the project options passed into that analysis.

[simolus3]

Instead of creating a fresh set for every column, could we make these a constant? E.g. use

const alwaysForbidden = {'hashCode', ...};
return drift3 ? const {...alwaysForbidden, 'col', ...} : const {...alwaysForbidden, 'sqliteAny', ...};

to avoid the 4x blowup, I think we can get away with unconditionally escaping moduleAndArgs.

[efim0v]

Done in 7453e0f, both sets are constants now. moduleAndArgs is reserved for every table, so the only visible change is that a module_and_args column in a regular table now gets moduleAndArgs$.

[simolus3]

With drift3, a column named map still breaks the toColumns method of the generated data class, which declares a local variable called map. That isn't related to inherited members.

Fixed in b622b36, thanks!

The schema tools analyze the SQL they rebuild tables from (schema dump on a database file, snapshots with fixed_sql) with default options rather than the project's, so with drift3 those tables are checked against the drift 2 members

I think that's acceptable, it'll get fixed once drift3 is released and the only option.

[efim0v]

You're welcome, and thanks for fixing toColumns so fast! Agreed on the schema tools.