JavaScript Invalid re-formatting

[davbro63]

This code produces an extra indent after the function header with default preferences:

function exit(code) {
        setTimeout(function() {
            phantom.exit(code);
        }, 0);
        phantom.onError = function() {};
    }
    // Comment

The expected formatting is:

function exit(code) {
    setTimeout(function() {
        phantom.exit(code);
    }, 0);
    phantom.onError = function() {};
}
// Comment

The incorrect formatting does not happen if the trailing // Comment is not present

[piranna]

I see this behaviour when using anonimous functions as callbacks when there's something after the function:

pc.setLocalDescription(answer, function () {
    console.log('Local description set', answer.sdp);

    callback(null, answer.sdp);
  },
  callback);

should be

pc.setLocalDescription(answer, function () {
  console.log('Local description set', answer.sdp);

  callback(null, answer.sdp);
},
callback);

[bitwiseman]

@piranna - What you're describing is a separate behavior that is by design.

These inputs remain unchanged. Everything inside the function() {...} is treated as one line, so the beautifier see these two statements as the same:

pc.setLocalDescription(answer, someFunction, callback);

pc.setLocalDescription(answer, function () {
  console.log('Local description set', answer.sdp);

  callback(null, answer.sdp);
}, callback);

When you put callback on a new line, the beautifier see it (basically) like either of these two, and indents to maintain clarity of the parameter list for setLocalDescription:

pc.setLocalDescription(answer, someFunction,
  callback);

pc.setLocalDescription(answer, 
  someFunction,
  callback);

pc.setLocalDescription(answer, function () {
    console.log('Local description set', answer.sdp);

    callback(null, answer.sdp);
  },
  callback);

[piranna]

I think both behaviour should be unified, and I propose to do so that if there's an inline anonimous function, they are aligned like in the first case (not indented). If not, you'll end up with things like this (real result):

id2object.call(self, error, result, operation, id, function (
  error, result) {
  if (error) return reject(error);

  resolve(result);
});

Here you indent one level due to the anonimous function arguments, but later you don't indent it one more time because the anonimous function block doesn't have an argument after it, so you get both arguments and block at the same level and it gets confuse. At least, the function header should be on a single line where possible:

id2object.call(self, error, result, operation, id,
function (error, result) {
  if (error) return reject(error);

  resolve(result);
});

Indented alternative:

id2object.call(self, error, result, operation, id,
  function (error, result) {
    if (error) return reject(error);

    resolve(result);
  });

Also, the function header should be in a new line if there are more arguments after it to don't get the "two indents bug", that honestly it's ugly and waste horizontal space, that makes when you have several nested callbacks the code get ugly due to line wrapping:

QUnit.test('currentFrame', function (assert) {
  var done = assert.async();

  assert.expect(2);

  var ctx = this

  function onerror(error) {
    if (error)
      QUnit.pushFailure(error.message || error, error.stack);

    done()
  }

  var video = document.getElementById('video')

  var options = {
    configuration: {
      iceServers: []
    },
    remoteVideo: video
  }

  ctx.webRtcPeer = WebRtcPeerRecvonly(options, function (error) {
    var self = this

    if (error) return onerror(error)

    this.generateOffer(function (error, sdpOffer, processAnswer) {
      if (error) return onerror(error)

      var offer = new RTCSessionDescription({
        type: 'offer',
        sdp: sdpOffer
      });

      ctx.peerConnection = new RTCPeerConnection()

      setIceCandidateCallbacks(this, ctx.peerConnection, onerror)

      ctx.peerConnection.setRemoteDescription(offer, function () {
          var mediaConstraints = {
            audio: false,
            fake: true,
            video: {
              mandatory: {
                maxWidth: 640,
                maxFrameRate: 15,
                minFrameRate: 15
              }
            }
          }

          getUserMedia(mediaConstraints, function (stream) {
              ctx.peerConnection.addStream(stream)

              ctx.peerConnection.createAnswer(function (answer) {
                  ctx.peerConnection.setLocalDescription(
                    answer,
                    function () {
                      processAnswer(answer.sdp, function (
                        error) {
                        if (error) return onerror(error)

                        var stream = this.getRemoteStream()
                        assert.notEqual(stream,
                          undefined, 'remote stream')

                        function onplaying() {
                          video.removeEventListener(
                            'playing', onplaying)

                          setTimeout(function () {
                            var currentFrame =
                              self.currentFrame

                            var x = currentFrame.width /
                              2
                            var y = currentFrame.height /
                              2

                            assert.notPixelEqual(
                              currentFrame, x,
                              y, 0, 0, 0, 0,
                              'playing');

                            done()
                          }, 1000)
                        }

                        video.addEventListener(
                          'playing', onplaying)
                      })
                    },
                    onerror);
                },
                onerror);
            },
            onerror)
        },
        onerror)
    })
  })
});

WTF?!?!

[bitwiseman]

@piranna - The comment after a function issue is a straight up bug. It will be fixed.
What you are requesting is an enhancement, or at very least a separate issue. Please take both your comments and open a new issue.

[cuixiping]

@bitwiseman Not only the comments after a function are removed, but also the comments before the function end are removed too.

function foo(/* comment-1 */ p1){ //comment-2
          //comment-3
    m();         //comment-4
    m();     //comment-5
               //comment-6
                       //comment-7
}
//comment-8
//comment-9

The expected formatting is:

function foo(/* comment-1 */ p1) { //comment-2
    //comment-3
    m(); //comment-4
    m(); //comment-5
    //comment-6
    //comment-7
}
//comment-8
//comment-9

but actually uglify-js result is:

function foo(/* comment-1 */ p1) {
    //comment-2
    //comment-3
    m();
    //comment-4
    m();
}

comment-5, 6, 7 dispeared.

[bitwiseman]

@cuixiping - This is not the uglify-js project. Please file your issue with them.

[bitwiseman]

This is a duplicate of #583.