argotorg / argotorg/solidity

Disallow trailing slashes in imports

Open
#10,980 5 comments 0 reactions 0 assignees View on GitHub
breaking change :warning: low effort low impact must have
Dominant language
C++
Stars
25.7k
Forks
6.2k
Avg merge
1d 11h
Merged PRs (30d)
21

Description

From https://github.com/ethereum/solidity/pull/10976#discussion_r578594268

## Abstract
Currently the compiler ignores any number of slashes at the end of the name of an imported module. For example this test case is perfectly valid:

```solidity
==== Source: a/b/c.sol ====
contract C {}
==== Source: a/b/d.sol ====
import "./c.sol/";
contract D is C {}
```

I think that this should result in a compilation error instead. Only `import "./c.sol";` should be accepted.

## Motivation
I see no legitimate need for these trailing slashes. I haven't seen this used in practice and if I did, I would probably be confused by it - Solidity does not currently allow importing whole directories but it looks like such an import.

It's also a source of bugs due to libraries not supporting all edge cases correctly (e.g. #10713). Disallowing this would free us from worrying about some of these edge cases.

## Specification
If the path passed to `import` ends with `/`, the compiler should issue an error saying that it's not possible to import a directory. For consistency, the compiler might also issue this error if it determines that an import not ending with a slash is pointing at a directory.

Alternatively, given that user might not understand that the compiler sees a file with a trailing slash as a directory, the message could just say that imported paths cannot end with a slash.

## Backwards Compatibility
This will break code that uses this style of import. Such code is easy to fix by removing the slash but it's still a breaking change.

Contributor guide

Open the contributing guide

Research direction

Start by tracing the compiler's handling of imported paths, using the Solidity import example in the issue as a regression case. Verify that an import ending in `/` is rejected with the intended diagnostic, while the equivalent path without the trailing slash remains accepted.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, solidity
Domain
compilers
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.