dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.11k stars 1.56k forks source link

Inconsistent behavior with case-insensitive file imports #56426

Open sebthom opened 1 month ago

sebthom commented 1 month ago

Description

I encountered surprising runtime issues in a project after renaming a Dart file from settings.dart to Settings.dart. The file was imported in several parts of my project, but I inadvertently failed to update the import statements in some places. Despite this, the project compiled and ran without errors, but I experienced unexpected runtime issues that were difficult and time-consuming to diagnose due to the project's complexity.

Root Cause Issue

Dart allows importing the same file using paths with different cases, which leads to multiple instances of that file being loaded into memory. This behavior causes problems, especially with static fields. For example, changes to a static variable in one instance of the file are not reflected in another instance if the file was imported with a different casing.

Proposed Solution

Dart should adopt a consistent approach to file imports:

The current behavior is problematic and violates the Principle of Least Astonishment (POLA), leading to confusing and hard-to-debug issues.

Environment

I am using the latest Dart version:

Dart SDK version: 3.5.0 (stable) (Tue Jul 30 02:17:59 2024 -0700) on "windows_x64"

Example

Settings.dart

int value = 0;

class Settings {
  static int value = 0;
}

main.dart

import './Settings.dart' as Settings1;
import './settings.dart' as Settings2;
import './SETTINGS.dart' as Settings3;

void main() {
  Settings1.value = 1;
  Settings1.Settings.value = 1;

  Settings2.value = 2;
  Settings2.Settings.value = 2;

  Settings3.value = 3;
  Settings3.Settings.value = 3;

  print(Settings1.value);          // prints 1 but at this point should print 3
  print(Settings1.Settings.value); // prints 1 but at this point should print 3

  print(Settings2.value);          // prints 2 but at this point should print 3
  print(Settings2.Settings.value); // prints 2 but at this point should print 3

  print(Settings3.value);          // prints 3
  print(Settings3.Settings.value); // prints 3
}
dart-github-bot commented 1 month ago

Summary: The user reports inconsistent behavior with case-insensitive file imports in Dart. Renaming a file from settings.dart to Settings.dart without updating all import statements resulted in unexpected runtime issues due to multiple instances of the file being loaded into memory, leading to inconsistent behavior with static fields.

lrhn commented 1 month ago

This is working as designed.

The Dart language is platform agnostic and identifies libraries by URI. URIs are case sensitive.

If imports exist for both import "Settings.dart"; and import "settings.dart", which end up being the same file in the (case-insensitive) file system, those will be different Dart libraries, which means they introduce different classes with the same name, that are not assignable to each other, and different instances of static variables.

The reason the compiler doesn't just say "those two files are obviously the same library" is that it's not always obvious. Some file systems are case sensitive, and two URIs that differ only in case may refer to different Dart files.

The compiler would have to know whether the file system is case sensitive, which can differ per directory on some platforms.

The only way to win is not to play. The compiler considers the URI the source of truth for library identity, and the file system as the source of truth for file content. That means that when you are on Windows or MacOS, with default case-insensitive file systems, your file system may allow import "settings.dart"; to read a file named Settings.dart, because the file system says that that is the content of the file settings.dart too. And if you try to compile the same code on a Unix system, or a Windows or MacOS system with a case-sensitive file system, it would fail to find a settings.dart file.

It is something that it might be reasonable to have a warning for. Generally, your Dart files should just be all lower-case ASCII, and the import URIs should match. That works everywhere. You can do other things, but then you can't assume that the program will compile on all platforms. Definitely do not have two entries in the same directory that differ only in case.

sebthom commented 1 month ago

@lrhn It seems Dart is taking a bit of a shortcut here. Even if the file system is case-insensitive, Dart could leverage methods like realpath or similar approaches to verify the actual case of the file on the file system and issue a compiler warning if there’s a mismatch or treat two imports with different casings to the same file as the same import.

Dart emphasizes static compilation, type- and null safety, yet this issue introduces a significant new class of bugs that are virtually non-existent in comparable static programming languages.

Furthermore, where is this behavior documented? There’s nothing about this peculiar behavior at dart.dev/language/libraries, nor any mention that imports should only use lowercase.

lrhn commented 1 month ago

There are things that can be done to detect problems. Dart probably should do some of those. It doesn't have to be perfect, as long as it catches the low-hanging fruits. (Two different non-empty libraries contaning the exact same content, that's probably a mistake worth checking. If they differ only in the case of their path, it's very likely a mistake.)

I'll reopen and make this a suggestion for the analyzer, which is the tool most likely to show warnings not required by the language.

Trying to correct for problems is worse, because a mistake can mean compiling the wrong code. Worst case, it succeeds.

The "use lower-case" suggestion is from the style guide.

sebthom commented 1 month ago

@lrhn Thanks for reconsideration. Is it worth opening an issue to enhance the documentation at dart.dev/language/libraries to include some information about the effects of case-insensitive imports?