Add '-export-fixes' command line option
AbandonedPublic

Authored by christiangagneraud on Wed, Apr 24, 10:38 PM.

Details

Reviewers
smartins
Summary

Couldn't get arc to push to the existing branch, instead it created another code review...
Is there an equivalent of git push, so that i can update my branch?

Test Plan

Tested with -checks=level1,level2,level3 -export-fixes=/tmp/clazy-fixes.yml -enable-all-fixits /home/chgans/Projects/navico/NS/nos/UI/tBlankScreen.cpp, here is an output excerpt:

- DiagnosticName:  clazy-qstring-allocations
  Message:         'QString(const char*) being called'
  FileOffset:      21477
  FilePath:        /home/chgans/Projects/navico/NS/nos/UI/tBlankScreen.cpp
  Replacements:    
    - FilePath:        /home/chgans/Projects/navico/NS/nos/UI/tBlankScreen.cpp
      Offset:          21495
      Length:          0
      ReplacementText: 'QLatin1String('
    - FilePath:        /home/chgans/Projects/navico/NS/nos/UI/tBlankScreen.cpp
      Offset:          21507
      Length:          0
      ReplacementText: ')'

There's currently 2 major issues:

  • FixItExporter::ConvertFixIt will screw up if macros are involved, shouldn't be hard to fix
  • If passing -export-fixes and several translation units, the exported file will be overwritten for each translation unit. I have to check how clang-tidy deals with this, but my feeling is that we should refuse to start in that case or we'll need to be able to aggregate clang::tooling::TranslationUnitDiagnostics, and this will have to be done outside the exporter, since a new context is created for each input to process

A minor issue is the handling of old vs new style to manage replacement, for now the code does one or the other, but doesn't warn or fail if both are request from the command line.

Diff Detail

Branch
yaml-export-fixes (branched from 406645-add-yaml-export-fixes)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 11203
Build 11221: arc lint + arc unit
christiangagneraud requested review of this revision.Wed, Apr 24, 10:38 PM
christiangagneraud created this revision.
christiangagneraud edited the summary of this revision. (Show Details)Wed, Apr 24, 10:45 PM
christiangagneraud added a reviewer: smartins.
christiangagneraud edited the summary of this revision. (Show Details)

D20799: Add '-export-fixes' command line option

christiangagneraud edited the summary of this revision. (Show Details)Wed, Apr 24, 11:59 PM
christiangagneraud edited the test plan for this revision. (Show Details)
christiangagneraud edited the summary of this revision. (Show Details)

Add diagnostic notes handling

Add diagnostic notes handling

can you just git push to the branch without arc ?

Maybe i don't have write access to the repo/branch?

chgans@chgans-pc:~/Projects/llvm-projects/clazy$ git remote -v
chgans  git@github.com:chgans/clazy.git (fetch)
chgans  git@github.com:chgans/clazy.git (push)
kde     git://anongit.kde.org/clazy (fetch)
kde     git://anongit.kde.org/clazy (push)
origin  https://github.com/KDE/clazy.git (fetch)
origin  https://github.com/KDE/clazy.git (push)
chgans@chgans-pc:~/Projects/llvm-projects/clazy$ git push -u kde kde-yaml-export-fixes:yaml-export-fixes 
fatal: remote error: service not enabled: /clazy

Or i got the remote wrong. that the only url given by phabricator. Do i need to join a project or something?

anongit is readonly, use git@git.kde.org:clazy instead

I've uploaded my public key to my profile, but it seems i don't have any rights to the repo/project/team/workspace:

chgans@chgans-pc:~/Projects/llvm-projects/clazy$ git remote -v
chgans  git@github.com:chgans/clazy.git (fetch)
chgans  git@github.com:chgans/clazy.git (push)
kde     git://anongit.kde.org/clazy (fetch)
kde     git://anongit.kde.org/clazy (push)
origin  https://github.com/KDE/clazy.git (fetch)
origin  https://github.com/KDE/clazy.git (push)
chgans@chgans-pc:~/Projects/llvm-projects/clazy$ git remote re
remove   rename   
chgans@chgans-pc:~/Projects/llvm-projects/clazy$ git remote rename kde ked-anon
chgans@chgans-pc:~/Projects/llvm-projects/clazy$ git remote add kde git@git.kde.org:clazy
chgans@chgans-pc:~/Projects/llvm-projects/clazy$ git fetch --all --prune
Fetching origin
Fetching chgans
Fetching ked-anon
Fetching kde
The authenticity of host 'git.kde.org (138.201.41.178)' can't be established.
ECDSA key fingerprint is SHA256:Bvp9zqBBiEf9QdpX09B0PL2CkDa1B64WtkKZyTq7XhM.
Are you sure you want to continue connecting (yes/no)? yes
Warning: Permanently added 'git.kde.org,138.201.41.178' (ECDSA) to the list of known hosts.
git@git.kde.org: Permission denied (publickey).
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
error: Could not fetch kde
chgans@chgans-pc:~/Projects/llvm-projects/clazy$

I have an account, but i have no rights on this repo it seems. Can you grant me access?

Can you push to your github ? I'll merge into clazy repo

just got my developer account, i've pushed with git push -u kde kde-yaml-export-fixes:yaml-export-fixes.
And i ended up with https://commits.kde.org/clazy/667d60e625382bcd9a254495b89da2dbc6cf5955

Am i by-passing code review now?

Am i by-passing code review now?

Yes, but that's fine since it went to a temporary feature branch. This way I can play with it directly and commit too. Will review before merging to master anyway!

christiangagneraud abandoned this revision.Sun, May 5, 9:23 PM

Has been integrated into master. Many thanks to @smartins