Skip to content
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

[linter][discarded_futures] false positive #59949

Open
stephane-archer opened this issue Jan 21, 2025 · 4 comments
Open

[linter][discarded_futures] false positive #59949

stephane-archer opened this issue Jan 21, 2025 · 4 comments
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. linter-false-positive P2 A bug or feature request we're likely to work on

Comments

@stephane-archer
Copy link

stephane-archer commented Jan 21, 2025

@immutable
class _LutFoldersState {
  final ISet<String> lutFolders;
  final PersistentLutFoldersSet persistentLutFoldersSet;
  final Future<List<DirectoryTree>> directoryTrees;
  final bool isInitialized;

  const _LutFoldersState(this.lutFolders, this.persistentLutFoldersSet,
      this.directoryTrees, this.isInitialized);

  _LutFoldersState.init()
      : lutFolders = ISet({}),
        persistentLutFoldersSet = PersistentLutFoldersSet(),
        directoryTrees = Future.value([]),
        isInitialized = false;

  _LutFoldersState copyWith({
    ISet<String>? lutFolders,
    PersistentLutFoldersSet? persistentLutFoldersSet,
    Future<List<DirectoryTree>>? directoryTrees,
    bool? isInitialized,
  }) {
    return _LutFoldersState(
      lutFolders ?? this.lutFolders,
      persistentLutFoldersSet ?? this.persistentLutFoldersSet,
      directoryTrees ??
          (lutFolders != null
              ? DirectoryTree.create(lutFolders.toSet())
              : this.directoryTrees),
      isInitialized ?? this.isInitialized,
    );
  }
}
Asynchronous function invoked in a non-'async' function.
Try converting the enclosing function to be 'async' and then 'await' the future.

The future created by DirectoryTree.create is not discarded, there is an assignment of the future.

@stephane-archer stephane-archer added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label Jan 21, 2025
@stephane-archer
Copy link
Author

@immutable
class _LutFoldersState {
  final ISet<String> lutFolders;
  final PersistentLutFoldersSet persistentLutFoldersSet;
  final Future<List<DirectoryTree>> directoryTrees; // TODO: remove future
  final bool isInitialized;

  const _LutFoldersState(this.lutFolders, this.persistentLutFoldersSet,
      this.directoryTrees, this.isInitialized);

  _LutFoldersState.init()
      : lutFolders = ISet({}),
        persistentLutFoldersSet = PersistentLutFoldersSet(),
        directoryTrees = Future.value([]),
        isInitialized = false;

  _LutFoldersState updateLutFolders(
    ISet<String> lutFolders,
  ) {
    return copyWith(
      lutFolders: lutFolders,
      // ignore: discarded_futures
      directoryTrees: DirectoryTree.create(lutFolders.toSet()),
    );
  }

  _LutFoldersState copyWith({
    ISet<String>? lutFolders,
    PersistentLutFoldersSet? persistentLutFoldersSet,
    Future<List<DirectoryTree>>? directoryTrees,
    bool? isInitialized,
  }) {
    return _LutFoldersState(
      lutFolders ?? this.lutFolders,
      persistentLutFoldersSet ?? this.persistentLutFoldersSet,
      directoryTrees ?? this.directoryTrees,
      isInitialized ?? this.isInitialized,
    );
  }
}

same issue here in updateLutFolders

@bwilkerson bwilkerson added P2 A bug or feature request we're likely to work on linter-false-positive labels Jan 21, 2025
@FMorschel
Copy link
Contributor

Hey, please take a look at #59887. There is a list of other issues that raise the same point about this lint. I'd consider this another duplicate. Either way, on that issue you can see a link to a CL that addresses this.

@stephane-archer
Copy link
Author

@FMorschel, thanks for looking at this issue. It looks quite similar. Have you tested the CL on the provided code?
Any assignment on a used variable should remove the current lint trigger. :)

@FMorschel
Copy link
Contributor

Have you tested the CL on the provided code?

As mentioned on #59887, I was still reviewing the test cases and the CL was still missing the binary expression (I do have a gist from Lasse that mentions every possibility for this that I'll take a look at yet, mentioned there). After handling binary expressions the ?? solved itself and there is no trigger for this anymore.

Any assignment on a used variable should remove the current lint trigger.

My current approach is that the variable must explicitly ask for a Future or FutureOr. This guarantees that the user (or API author) knows that is a Future and will most-likely handle it correctly. So if you do var f = myFunctionThatReturnsFuture() it will trigger the lint unless you write Future.

Your suggestion for handling this is another valid option still, but I'm unsure of what the team will prefer. If you fell like your suggestion is better, please feel free and comment on the mentioned issue so we can focus all discussions for it there.


on the provided code

A small side-note. Although I was able to add some code (below) to make your code stop having errors, I'd advise that for future issues (unless you really don't know how - there are cases where this is hard see #56911 and #59653 threads as an example) you provide a more concise code, containing everything necessary and removing boilerplate. This will help the team to debug your issues faster and they also can use your example as the test case.

What I had to add to your code:

extension type ISet<T>(Set<T> set) implements Set<T> {
  Set<T> toSet() => set;
}

class PersistentLutFoldersSet {}

class DirectoryTree {
  static Future<List<DirectoryTree>> create(Set<String> lutFolders) async {
    return [];
  }
}

What I managed to create as an example of the false-positive you found:

void foo() {
  Future<int>? variable;
  Future<int> _ = variable ?? g();
}

Future<int> g() async => 0;

It'd be even more helpful if you could explain where is the false-positive happening (as you did) like saying "On line 3 you see the lint under ..." and maybe add a comment on the code (even more so when the repro is large) so it stands out on the code block like your ignore on the first comment. Something like that will make fixing future issues easier and possibly faster since the other person won't need to run the code locally (or try to find the place you mentioned inside a larger example) to see/understand what is happening first 😁.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. linter-false-positive P2 A bug or feature request we're likely to work on
Projects
None yet
Development

No branches or pull requests

3 participants