avh4 / elm-format

elm-format formats Elm source code according to a standard set of rules based on the official Elm Style Guide
BSD 3-Clause "New" or "Revised" License
1.31k stars 148 forks source link

Remove unused imports #135

Closed gyzerok closed 6 years ago

gyzerok commented 8 years ago

Sorry if this feature already requested. I can't find issue.

avh4 commented 8 years ago

This is similar to #138 in that it requires information from the compiler to know which imports are unused. Because of that, I'm postponing considering it until after 1.0 is released.

There is also #8 about sorting imports alphabetically.

A goal for elm-format is to have it be set to format-on-save for the majority of people using it. Removing unused imports could cause some annoyance while developing if you added an import and then format before writing the code that uses the import.

benjick commented 8 years ago

Wouldn't this be weird on refactor or on splitting files when you're trying stuff out? You might not need it on that save, but on the next one

onemanstartup commented 8 years ago

It is not formatting, it should be handled by static analysis tools

mgold commented 8 years ago

Agree, this feels too close to messing with semantics for a formatting tool. Maybe if it was a command-line argument, i.e. not something that happened every save. But that doesn't seem high priority.

liamcurry commented 8 years ago

It'd be cool if imports were automatically removed/added/managed like goimports does with gofmt.

trotha01 commented 8 years ago

+1 to @liamcurry's comment. I love that I don't have to worry about listing imports at the top of the file with go projects. This also means that 'exposing (..)' would never be needed if a tool does the imports for you.

I agree though that this might be better as a separate tool, similar to how gofmt and goimports are separate.

PedroHLC commented 6 years ago

Though this is an old and frozen discussion, did anyone implemented this feature? Some fork I can rely on? My code base is rather large, and we missed a lot of unused imports along the way, it will take eternity to remove every one manually...

rtfeldman commented 6 years ago

Removing unused imports could cause some annoyance while developing if you added an import and then format before writing the code that uses the import.

I experienced this back in my Java programming days. When I'd save, my IDE would delete unused imports.

It was super annoying. Whenever I'd comment out something and save, it would delete the imports that were required by that code. Later when I came back to uncomment the code, it wouldn't work anymore - I'd have to re-add the imports.

From my perspective, this should be a feature request for editor plugins (the Elm plugin for already has Quick Fix All, which removes unused imports), not for elm-format.

gyzerok commented 6 years ago

Yep, that makes sense to me. Ideally I would want compiler to point me out about unused imports.

chrisfls commented 6 years ago

Agree, this feels too close to messing with semantics for a formatting tool. Maybe if it was a command-line argument, i.e. not something that happened every save. But that doesn't seem high priority.

It'd be cool if imports were automatically removed/added/managed like goimports does with gofmt.

👍 but it's understandable why not including it here (looks like you guys want elm-format to fix the code format without touching code semantics)

jinjor commented 6 years ago

I managed to write a script to do it, something like this.

remove-unused-imports.sh

#!/bin/bash

report_file=./report.txt

rm -rf elm-stuff/build-artifacts &&
  elm-make src/elm/Main.elm --output=/dev/null --warn --yes --report=json > $report_file &&
  node ./tools/remove-unused-imports $report_file &&
  rm $report_file &&
  elm-format src/ --yes

remove-unused-imports.js

const fs = require("fs");

const reportFile = process.argv[2];
fs
  .readFileSync(reportFile, "utf8")
  .split("\n")
  .filter(line => {
    return line.charAt(0) === "[";
  })
  .map(line => {
    const warnings = JSON.parse(line);
    const file = warnings[0].file;
    const contentArray = fs.readFileSync(file, "utf8").split("\n");
    warnings
      .filter(w => {
        return w.tag === "unused import";
      })
      .forEach(w => {
        for (let i = w.region.start.line - 1; i <= w.region.end.line - 1; i++) {
          contentArray[i] = "";
        }
      });
    fs.writeFileSync(file, contentArray.join("\n"));
  });

I'll try it in my project and see if it is useful :)

andys8 commented 6 years ago

@jinjor Have you thought about putting it on npm as a little helper package for commandline use for everyone? elm-remove-unused-imports? Would be awesome.

mordrax commented 6 years ago

This package did it for me with 0.18 elm files: https://www.npmjs.com/package/elm-impfix

recompiled 394 files, seems to be working :)