Skip to content

Generated code can contain unused variables #3384

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
PIG208 opened this issue Dec 19, 2024 · 6 comments
Closed

Generated code can contain unused variables #3384

PIG208 opened this issue Dec 19, 2024 · 6 comments

Comments

@PIG208
Copy link
Contributor

PIG208 commented Dec 19, 2024

Describe the bug

The generated validateIntegrity method can contain a final data = instance.toColumns(true);
statement that defines an unused local variable.

This can be reproduced by having a table that contains only columns with a type converter.

For example:

enum Fruit {
  apple,
  pear,
}
class Foo extends Table {
  Column<String> get fruit => textEnum<Fruit>()
    .withDefault(const Variable('apple'))();
}
  @override
  VerificationContext validateIntegrity(Insertable<FooData> instance,
      {bool isInserting = false}) {
    final context = VerificationContext();
    final data = instance.toColumns(true); // The value of the local variable 'data' isn't used
    context.handle(_fruitMeta, const VerificationResult.success());
    return context;
  }

We don't necessarily need to avoid generating code like this. Adding the lint rule to ignore_for_file should fix this issue.

@simolus3
Copy link
Owner

Hm, there should be an //ignore_for_file: type=lint comment at the top of the file, but I guess that doesn't catch dead code warnings?

Still, good catch! It's not hard to just not generate that effectively empty method in that case.

@gnprice
Copy link
Contributor

gnprice commented Dec 19, 2024

Thanks for the swift fix!

I believe type=lint covers the rules that appear here:
https://dart.dev/tools/linter-rules

I'm not sure what specific warning was appearing in this issue, but on a quick search for "unused" it doesn't look like that list includes one about unused variables. So it may be a Dart analyzer warning that isn't one of the "linter" rules, and therefore isn't covered by type=lint.

@PIG208
Copy link
Contributor Author

PIG208 commented Feb 20, 2025

Generated file contains unused variable when the table contains only a textEnum. Maybe we should reopen this issue? @simolus3

class GlobalSettings extends Table {
  Column<String> get themeSetting => textEnum<ThemeSetting>()();
}
class $GlobalSettingsTable extends GlobalSettings
    with TableInfo<$GlobalSettingsTable, GlobalSetting> {
  @override
  final GeneratedDatabase attachedDatabase;
  final String? _alias;
  $GlobalSettingsTable(this.attachedDatabase, [this._alias]);
  // Warning: The value of the field '_themeSettingMeta' isn't used.
  static const VerificationMeta _themeSettingMeta = const VerificationMeta(
    'themeSetting',
  );
  @override
  late final GeneratedColumnWithTypeConverter<ThemeSetting, String>
  themeSetting = GeneratedColumn<String>(
    'theme_setting',
    aliasedName,
    false,
    type: DriftSqlType.string,
    requiredDuringInsert: true,
  ).withConverter<ThemeSetting>($GlobalSettingsTable.$converterthemeSetting);
  @override
  List<GeneratedColumn> get $columns => [themeSetting];
  @override
  String get aliasedName => _alias ?? actualTableName;
  @override
  String get actualTableName => $name;
  static const String $name = 'global_settings';
  @override
  Set<GeneratedColumn> get $primaryKey => const {};
  @override
  GlobalSetting map(Map<String, dynamic> data, {String? tablePrefix}) {
    final effectivePrefix = tablePrefix != null ? '$tablePrefix.' : '';
    return GlobalSetting(
      themeSetting: $GlobalSettingsTable.$converterthemeSetting.fromSql(
        attachedDatabase.typeMapping.read(
          DriftSqlType.string,
          data['${effectivePrefix}theme_setting'],
        )!,
      ),
    );
  }

If another column is added to the table, or the column builder is changed to other things like text or integer, there is no longer this issue.

It seems like the fix #3384 (comment) needs to further drop VerificationMeta for fields like this when validateIntegrity is not generated.

Reproduced on 2.25.1.

@simolus3
Copy link
Owner

Thanks for the report, I've fixed this in 8b8aacb.

@PIG208
Copy link
Contributor Author

PIG208 commented Mar 4, 2025

Hi @simolus3. Thanks a lot for the prompt fix!

Do you have an estimate of when the next release will come out?

(It's not necessarily blocking us as we can pin a Git revision containing the fix for now)

@simolus3
Copy link
Owner

simolus3 commented Mar 5, 2025

I'll try to do a release this weekend, there are some other minor patches I want to look at too in the meantime.

PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Mar 11, 2025
This contains a fix for simolus3/drift#3384.

Signed-off-by: Zixuan James Li <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants